diff --git a/pkg/actuators/machine/actuator.go b/pkg/actuators/machine/actuator.go index ace4375f25..816859ba5a 100644 --- a/pkg/actuators/machine/actuator.go +++ b/pkg/actuators/machine/actuator.go @@ -25,7 +25,6 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine" - mapierrors "github.com/openshift/machine-api-operator/pkg/controller/machine" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" apimachineryerrors "k8s.io/apimachinery/pkg/api/errors" @@ -33,7 +32,6 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" "k8s.io/klog" - "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1beta1" providerconfigv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1beta1" awsclient "sigs.k8s.io/cluster-api-provider-aws/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client" @@ -237,7 +235,7 @@ func (a *Actuator) setMachineProviderConditions(machine *machinev1.Machine, cond func (a *Actuator) CreateMachine(machine *machinev1.Machine) (*ec2.Instance, error) { machineProviderConfig, err := providerConfigFromMachine(machine, a.codec) if err != nil { - return nil, a.handleMachineError(machine, mapierrors.InvalidMachineConfiguration("error decoding MachineProviderConfig: %v", err), createEventAction) + return nil, a.handleMachineError(machine, machinecontroller.InvalidMachineConfiguration("error decoding MachineProviderConfig: %v", err), createEventAction) } credentialsSecretName := "" @@ -295,13 +293,13 @@ func (a *Actuator) getUserData(machine *machinev1.Machine, machineProviderConfig err := a.client.Get(context.Background(), client.ObjectKey{Namespace: machine.Namespace, Name: machineProviderConfig.UserDataSecret.Name}, &userDataSecret) if err != nil { if apimachineryerrors.IsNotFound(err) { - return nil, a.handleMachineError(machine, mapierrors.InvalidMachineConfiguration("user data secret %s: %v not found", machineProviderConfig.UserDataSecret.Name, err), createEventAction) + return nil, a.handleMachineError(machine, machinecontroller.InvalidMachineConfiguration("user data secret %s: %v not found", machineProviderConfig.UserDataSecret.Name, err), createEventAction) } - return nil, a.handleMachineError(machine, mapierrors.CreateMachine("error getting user data secret %s: %v", machineProviderConfig.UserDataSecret.Name, err), createEventAction) + return nil, a.handleMachineError(machine, machinecontroller.CreateMachine("error getting user data secret %s: %v", machineProviderConfig.UserDataSecret.Name, err), createEventAction) } userData, exists := userDataSecret.Data[userDataSecretKey] if !exists { - return nil, a.handleMachineError(machine, mapierrors.InvalidMachineConfiguration("%s: Secret %v/%v does not have %q field set. Thus, no user data applied when creating an instance.", machine.Name, machine.Namespace, machineProviderConfig.UserDataSecret.Name, userDataSecretKey), createEventAction) + return nil, a.handleMachineError(machine, machinecontroller.InvalidMachineConfiguration("%s: Secret %v/%v does not have %q field set. Thus, no user data applied when creating an instance.", machine.Name, machine.Namespace, machineProviderConfig.UserDataSecret.Name, userDataSecretKey), createEventAction) } return userData, nil } @@ -320,7 +318,7 @@ func (a *Actuator) Delete(context context.Context, machine *machinev1.Machine) e func (a *Actuator) DeleteMachine(machine *machinev1.Machine) error { machineProviderConfig, err := providerConfigFromMachine(machine, a.codec) if err != nil { - return a.handleMachineError(machine, mapierrors.InvalidMachineConfiguration("error decoding MachineProviderConfig: %v", err), deleteEventAction) + return a.handleMachineError(machine, machinecontroller.InvalidMachineConfiguration("error decoding MachineProviderConfig: %v", err), deleteEventAction) } region := machineProviderConfig.Placement.Region @@ -348,7 +346,7 @@ func (a *Actuator) DeleteMachine(machine *machinev1.Machine) error { terminatingInstances, err := terminateInstances(client, existingInstances) if err != nil { - return a.handleMachineError(machine, mapierrors.DeleteMachine(err.Error()), noEventAction) + return a.handleMachineError(machine, machinecontroller.DeleteMachine(err.Error()), noEventAction) } if len(terminatingInstances) == 1 { @@ -374,7 +372,7 @@ func (a *Actuator) Update(context context.Context, machine *machinev1.Machine) e machineProviderConfig, err := providerConfigFromMachine(machine, a.codec) if err != nil { - return a.handleMachineError(machine, mapierrors.InvalidMachineConfiguration("error decoding MachineProviderConfig: %v", err), updateEventAction) + return a.handleMachineError(machine, machinecontroller.InvalidMachineConfiguration("error decoding MachineProviderConfig: %v", err), updateEventAction) } region := machineProviderConfig.Placement.Region @@ -406,7 +404,7 @@ func (a *Actuator) Update(context context.Context, machine *machinev1.Machine) e klog.Warningf("%s: attempted to update machine but no instances found", machine.Name) - a.handleMachineError(machine, mapierrors.UpdateMachine("no instance found, reason unknown"), updateEventAction) + a.handleMachineError(machine, machinecontroller.UpdateMachine("no instance found, reason unknown"), updateEventAction) // Update status to clear out machine details. if err := a.setStatus(machine, nil, conditionSuccess()); err != nil { @@ -428,7 +426,7 @@ func (a *Actuator) Update(context context.Context, machine *machinev1.Machine) e err = a.updateLoadBalancers(client, machineProviderConfig, newestInstance, machine.Name) if err != nil { - a.handleMachineError(machine, mapierrors.CreateMachine("Error updating load balancers: %v", err), updateEventAction) + a.handleMachineError(machine, machinecontroller.CreateMachine("Error updating load balancers: %v", err), updateEventAction) return err } } else { @@ -569,7 +567,7 @@ func (a *Actuator) updateLoadBalancers(client awsclient.Client, providerConfig * } // setStatus calculates the new machine status, checks if anything has changed, and updates if so. -func (a *Actuator) setStatus(machine *machinev1.Machine, instance *ec2.Instance, condition v1beta1.AWSMachineProviderCondition) error { +func (a *Actuator) setStatus(machine *machinev1.Machine, instance *ec2.Instance, condition providerconfigv1.AWSMachineProviderCondition) error { klog.Infof("%s: Updating status", machine.Name) // Starting with a fresh status as we assume full control of it here. diff --git a/pkg/actuators/machine/actuator_test.go b/pkg/actuators/machine/actuator_test.go index a5c4b985c2..013c7b2e33 100644 --- a/pkg/actuators/machine/actuator_test.go +++ b/pkg/actuators/machine/actuator_test.go @@ -16,7 +16,6 @@ import ( machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" machineapierrors "github.com/openshift/machine-api-operator/pkg/controller/machine" "github.com/stretchr/testify/assert" - apiv1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -724,11 +723,11 @@ func TestGetUserData(t *testing.T) { } testCases := []struct { - secret *apiv1.Secret + secret *corev1.Secret error error }{ { - secret: &apiv1.Secret{ + secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: userDataSecretName, Namespace: defaultNamespace, @@ -740,7 +739,7 @@ func TestGetUserData(t *testing.T) { error: nil, }, { - secret: &apiv1.Secret{ + secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "notFound", Namespace: defaultNamespace, @@ -752,7 +751,7 @@ func TestGetUserData(t *testing.T) { error: &machineapierrors.MachineError{}, }, { - secret: &apiv1.Secret{ + secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: userDataSecretName, Namespace: defaultNamespace, @@ -933,7 +932,7 @@ func TestCreate(t *testing.T) { }, }, }, - userDataSecret: &apiv1.Secret{ + userDataSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: userDataSecretName, Namespace: defaultNamespace, diff --git a/pkg/actuators/machine/utils.go b/pkg/actuators/machine/utils.go index 72723202c5..23e5f0432a 100644 --- a/pkg/actuators/machine/utils.go +++ b/pkg/actuators/machine/utils.go @@ -239,19 +239,16 @@ func (a *Actuator) isMaster(machine *machinev1.Machine) (bool, error) { // setAWSMachineProviderCondition sets the condition for the machine and // returns the new slice of conditions. // If the machine does not already have a condition with the specified type, -// a condition will be added to the slice if and only if the specified -// status is True. +// a condition will be added to the slice // If the machine does already have a condition with the specified type, // the condition will be updated if either of the following are true. func setAWSMachineProviderCondition(condition providerconfigv1.AWSMachineProviderCondition, conditions []providerconfigv1.AWSMachineProviderCondition) []providerconfigv1.AWSMachineProviderCondition { now := metav1.Now() if existingCondition := findProviderCondition(conditions, condition.Type); existingCondition == nil { - if condition.Status == corev1.ConditionTrue { - condition.LastProbeTime = now - condition.LastTransitionTime = now - conditions = append(conditions, condition) - } + condition.LastProbeTime = now + condition.LastTransitionTime = now + conditions = append(conditions, condition) } else { updateExistingCondition(&condition, existingCondition) } @@ -364,7 +361,7 @@ func conditionSuccess() providerconfigv1.AWSMachineProviderCondition { func conditionFailed() providerconfigv1.AWSMachineProviderCondition { return providerconfigv1.AWSMachineProviderCondition{ Type: providerconfigv1.MachineCreation, - Status: corev1.ConditionTrue, + Status: corev1.ConditionFalse, Reason: providerconfigv1.MachineCreationFailed, } } diff --git a/pkg/apis/awsproviderconfig/v1beta1/awsmachineproviderconfig_types.go b/pkg/apis/awsproviderconfig/v1beta1/awsmachineproviderconfig_types.go index c5ce98b5b6..5eef5d25a3 100644 --- a/pkg/apis/awsproviderconfig/v1beta1/awsmachineproviderconfig_types.go +++ b/pkg/apis/awsproviderconfig/v1beta1/awsmachineproviderconfig_types.go @@ -53,24 +53,24 @@ type AWSMachineProviderStatus struct { // AWSMachineProviderConditionType is a valid value for AWSMachineProviderCondition.Type type AWSMachineProviderConditionType string -// Valid conditions for an AWS machine instance +// Valid conditions for an AWS machine instance. const ( // MachineCreation indicates whether the machine has been created or not. If not, // it should include a reason and message for the failure. MachineCreation AWSMachineProviderConditionType = "MachineCreation" ) -// AWSMachineProviderConditionReason is reason for the condition's last transition +// AWSMachineProviderConditionReason is reason for the condition's last transition. type AWSMachineProviderConditionReason string const ( - // MachineCreationSucceeded indicates machine creation success + // MachineCreationSucceeded indicates machine creation success. MachineCreationSucceeded AWSMachineProviderConditionReason = "MachineCreationSucceeded" - // MachineCreationFailed indicates machine creation fail + // MachineCreationFailed indicates machine creation failure. MachineCreationFailed AWSMachineProviderConditionReason = "MachineCreationFailed" ) -// AWSMachineProviderCondition is a condition in a AWSMachineProviderStatus +// AWSMachineProviderCondition is a condition in a AWSMachineProviderStatus. type AWSMachineProviderCondition struct { // Type is the type of the condition. Type AWSMachineProviderConditionType `json:"type"`