Skip to content

Commit

Permalink
Remove some use of deprecated errors package
Browse files Browse the repository at this point in the history
  • Loading branch information
mboersma committed Dec 2, 2024
1 parent d5520b8 commit c9f128b
Show file tree
Hide file tree
Showing 18 changed files with 80 additions and 101 deletions.
6 changes: 1 addition & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,7 @@ CODESPELL_BIN := codespell
CODESPELL_DIST_DIR := codespell_dist
CODESPELL := $(TOOLS_BIN_DIR)/$(CODESPELL_DIST_DIR)/$(CODESPELL_BIN)

# This is a commit from CR main (22.05.2024).
# Intentionally using a commit from main to use a setup-envtest version
# that uses binaries from controller-tools, not GCS.
# CR PR: https://github.com/kubernetes-sigs/controller-runtime/pull/2811
SETUP_ENVTEST_VER := v0.0.0-20240522175850-2e9781e9fc60
SETUP_ENVTEST_VER := release-0.19
SETUP_ENVTEST_BIN := setup-envtest
SETUP_ENVTEST := $(abspath $(TOOLS_BIN_DIR)/$(SETUP_ENVTEST_BIN)-$(SETUP_ENVTEST_VER))
SETUP_ENVTEST_PKG := sigs.k8s.io/controller-runtime/tools/setup-envtest
Expand Down
3 changes: 1 addition & 2 deletions api/v1beta1/azuremachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/errors" //nolint:staticcheck
)

const (
Expand Down Expand Up @@ -219,7 +218,7 @@ type AzureMachineStatus struct {
// can be added as events to the Machine object and/or logged in the
// controller's output.
// +optional
FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"`
FailureReason *string `json:"failureReason,omitempty"`

// ErrorMessage will be set in the event that there is a terminal problem
// reconciling the Machine and will contain a more verbose string suitable
Expand Down
3 changes: 1 addition & 2 deletions api/v1beta1/azuremanagedmachinepool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package v1beta1
import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck
)

const (
Expand Down Expand Up @@ -464,7 +463,7 @@ type AzureManagedMachinePoolStatus struct {
// can be added as events to the Machine object and/or logged in the
// controller's output.
// +optional
ErrorReason *capierrors.MachineStatusError `json:"errorReason,omitempty"`
ErrorReason *string `json:"errorReason,omitempty"`

// Any transient errors that occur during the reconciliation of Machines
// can be added as events to the Machine object and/or logged in the
Expand Down
5 changes: 2 additions & 3 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions azure/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
Expand Down Expand Up @@ -600,7 +599,7 @@ func (m *MachineScope) SetFailureMessage(v error) {
}

// SetFailureReason sets the AzureMachine status failure reason.
func (m *MachineScope) SetFailureReason(v capierrors.MachineStatusError) {
func (m *MachineScope) SetFailureReason(v string) {

Check warning on line 602 in azure/scope/machine.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/machine.go#L602

Added line #L602 was not covered by tests
m.AzureMachine.Status.FailureReason = &v
}

Expand Down
3 changes: 1 addition & 2 deletions azure/scope/machinepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
Expand Down Expand Up @@ -648,7 +647,7 @@ func (m *MachinePoolScope) SetFailureMessage(v error) {
}

// SetFailureReason sets the AzureMachinePool status failure reason.
func (m *MachinePoolScope) SetFailureReason(v capierrors.MachineStatusError) {
func (m *MachinePoolScope) SetFailureReason(v string) {

Check warning on line 650 in azure/scope/machinepool.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/machinepool.go#L650

Added line #L650 was not covered by tests
m.AzureMachinePool.Status.FailureReason = &v
}

Expand Down
3 changes: 1 addition & 2 deletions azure/scope/machinepoolmachine.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/controllers/noderefutil"
"sigs.k8s.io/cluster-api/controllers/remote"
capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
Expand Down Expand Up @@ -277,7 +276,7 @@ func (s *MachinePoolMachineScope) SetFailureMessage(v error) {
}

// SetFailureReason sets the AzureMachinePoolMachine status failure reason.
func (s *MachinePoolMachineScope) SetFailureReason(v capierrors.MachineStatusError) {
func (s *MachinePoolMachineScope) SetFailureReason(v string) {

Check warning on line 279 in azure/scope/machinepoolmachine.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/machinepoolmachine.go#L279

Added line #L279 was not covered by tests
s.AzureMachinePoolMachine.Status.FailureReason = &v
}

Expand Down
3 changes: 1 addition & 2 deletions azure/services/aso/aso.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,7 @@ func isOwnedBy(resource client.Object, owner client.Object, scheme *runtime.Sche
}

func hasLegacyOwnedByLabel(labels map[string]string, clusterName string) bool {
//nolint:staticcheck // Referencing this deprecated value is required for backwards compatibility.
return labels[infrav1.OwnedByClusterLabelKey] == clusterName
return labels[infrav1.OwnedByClusterLabelKey] == clusterName //nolint:staticcheck
}

// PauseResource pauses an ASO resource by updating its `reconcile-policy` to `skip`.
Expand Down
5 changes: 2 additions & 3 deletions azure/services/aso/aso_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -565,9 +565,8 @@ func TestCreateOrUpdateResource(t *testing.T) {
Name: "name",
Namespace: "namespace",
Labels: map[string]string{
clusterv1.ClusterNameLabel: clusterName,
//nolint:staticcheck // Referencing this deprecated value is required for backwards compatibility.
infrav1.OwnedByClusterLabelKey: clusterName,
clusterv1.ClusterNameLabel: clusterName,
infrav1.OwnedByClusterLabelKey: clusterName, //nolint:staticcheck
},
Annotations: map[string]string{
asoannotations.ReconcilePolicy: string(asoannotations.ReconcilePolicySkip),
Expand Down
5 changes: 2 additions & 3 deletions controllers/azurecluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"k8s.io/client-go/tools/record"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
Expand All @@ -51,7 +50,7 @@ import (
type TestClusterReconcileInput struct {
createAzureClusterService func(*scope.ClusterScope) (*azureClusterService, error)
azureClusterOptions func(ac *infrav1.AzureCluster)
clusterScopeFailureReason capierrors.ClusterStatusError
clusterScopeFailureReason string
cache *scope.ClusterCache
expectedResult reconcile.Result
expectedErr string
Expand Down Expand Up @@ -182,7 +181,7 @@ func TestAzureClusterReconcileNormal(t *testing.T) {
acs.scope = cs
}), nil
},
clusterScopeFailureReason: capierrors.CreateClusterError,
clusterScopeFailureReason: "CreateError",
cache: &scope.ClusterCache{},
},
"should requeue if transient error is received": {
Expand Down
9 changes: 4 additions & 5 deletions controllers/azuremachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/tools/record"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/conditions"
Expand Down Expand Up @@ -277,7 +276,7 @@ func (amr *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineS
if errors.As(err, &reconcileError) && reconcileError.IsTerminal() {
amr.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "SKUNotFound", errors.Wrap(err, "failed to initialize machine cache").Error())
log.Error(err, "Failed to initialize machine cache")
machineScope.SetFailureReason(capierrors.InvalidConfigurationMachineError)
machineScope.SetFailureReason("InvalidConfiguration")

Check warning on line 279 in controllers/azuremachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/azuremachine_controller.go#L279

Added line #L279 was not covered by tests
machineScope.SetFailureMessage(err)
machineScope.SetNotReady()
return reconcile.Result{}, nil
Expand All @@ -289,7 +288,7 @@ func (amr *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineS
cond := conditions.Get(machineScope.AzureMachine, infrav1.VMIdentitiesReadyCondition)
if cond != nil && cond.Status == corev1.ConditionFalse && cond.Reason == infrav1.UserAssignedIdentityMissingReason {
amr.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, infrav1.UserAssignedIdentityMissingReason, "VM is unhealthy")
machineScope.SetFailureReason(capierrors.UnsupportedChangeMachineError)
machineScope.SetFailureReason("UnsupportedChange")
machineScope.SetFailureMessage(errors.New("VM identities are not ready"))
return reconcile.Result{}, errors.New("VM identities are not ready")
}
Expand All @@ -304,7 +303,7 @@ func (amr *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineS
// In this case, we mark it as failed and leave it to MHC for remediation
if errors.As(err, &azure.VMDeletedError{}) {
amr.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "VMDeleted", errors.Wrap(err, "failed to reconcile AzureMachine").Error())
machineScope.SetFailureReason(capierrors.UpdateMachineError)
machineScope.SetFailureReason("UpdateError")
machineScope.SetFailureMessage(err)
machineScope.SetNotReady()
machineScope.SetVMState(infrav1.Deleted)
Expand All @@ -316,7 +315,7 @@ func (amr *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineS
if reconcileError.IsTerminal() {
amr.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "ReconcileError", errors.Wrapf(err, "failed to reconcile AzureMachine").Error())
log.Error(err, "failed to reconcile AzureMachine", "name", machineScope.Name())
machineScope.SetFailureReason(capierrors.CreateMachineError)
machineScope.SetFailureReason("CreateError")
machineScope.SetFailureMessage(err)
machineScope.SetNotReady()
machineScope.SetVMState(infrav1.Failed)
Expand Down
10 changes: 4 additions & 6 deletions controllers/azuremachine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"k8s.io/client-go/tools/record"
"k8s.io/utils/ptr"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
Expand All @@ -47,7 +46,7 @@ type TestMachineReconcileInput struct {
createAzureMachineService func(*scope.MachineScope) (*azureMachineService, error)
azureMachineOptions func(am *infrav1.AzureMachine)
expectedErr string
machineScopeFailureReason capierrors.MachineStatusError
machineScopeFailureReason string
ready bool
cache *scope.MachineCache
skuCache scope.SKUCacher
Expand Down Expand Up @@ -185,8 +184,7 @@ func TestAzureMachineReconcileNormal(t *testing.T) {
},
"should skip reconciliation if error state is detected on azure machine": {
azureMachineOptions: func(am *infrav1.AzureMachine) {
updateError := capierrors.UpdateMachineError
am.Status.FailureReason = &updateError
am.Status.FailureReason = ptr.To("UpdateError")
},
createAzureMachineService: getFakeAzureMachineService,
},
Expand Down Expand Up @@ -219,13 +217,13 @@ func TestAzureMachineReconcileNormal(t *testing.T) {
},
"should fail if VM is deleted": {
createAzureMachineService: getFakeAzureMachineServiceWithVMDeleted,
machineScopeFailureReason: capierrors.UpdateMachineError,
machineScopeFailureReason: "UpdateError",
cache: &scope.MachineCache{},
expectedErr: "failed to reconcile AzureMachine",
},
"should reconcile if terminal error is received": {
createAzureMachineService: getFakeAzureMachineServiceWithTerminalError,
machineScopeFailureReason: capierrors.CreateMachineError,
machineScopeFailureReason: "CreateError",
cache: &scope.MachineCache{},
},
"should requeue if transient error is received": {
Expand Down
Loading

0 comments on commit c9f128b

Please sign in to comment.