diff --git a/pkg/apis/awsproviderconfig/v1alpha1/register.go b/pkg/apis/awsproviderconfig/v1alpha1/register.go index bc2f4a0dd4..6df05ad742 100644 --- a/pkg/apis/awsproviderconfig/v1alpha1/register.go +++ b/pkg/apis/awsproviderconfig/v1alpha1/register.go @@ -27,6 +27,7 @@ package v1alpha1 import ( "bytes" "fmt" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" @@ -103,7 +104,7 @@ func (codec *AWSProviderConfigCodec) EncodeProviderStatus(in runtime.Object) (*r return &runtime.RawExtension{Raw: buf.Bytes()}, nil } -// DecodeProviderStatus serialises the provider status +// DecodeProviderStatus deserialises the provider status func (codec *AWSProviderConfigCodec) DecodeProviderStatus(providerStatus *runtime.RawExtension, out runtime.Object) error { if providerStatus != nil { _, _, err := codec.decoder.Decode(providerStatus.Raw, nil, out) diff --git a/pkg/cloud/aws/actuators/machine/actuator.go b/pkg/cloud/aws/actuators/machine/actuator.go index 674e47f8d8..a183773396 100644 --- a/pkg/cloud/aws/actuators/machine/actuator.go +++ b/pkg/cloud/aws/actuators/machine/actuator.go @@ -37,7 +37,6 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" - "k8s.io/apimachinery/pkg/runtime" awsclient "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/client" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -62,8 +61,9 @@ type Actuator struct { kubeClient kubernetes.Interface client client.Client awsClientBuilder awsclient.AwsClientBuilderFuncType - codec codec - eventRecorder record.EventRecorder + + codec *providerconfigv1.AWSProviderConfigCodec + eventRecorder record.EventRecorder } // ActuatorParams holds parameter information for Actuator @@ -71,16 +71,10 @@ type ActuatorParams struct { KubeClient kubernetes.Interface Client client.Client AwsClientBuilder awsclient.AwsClientBuilderFuncType - Codec codec + Codec *providerconfigv1.AWSProviderConfigCodec EventRecorder record.EventRecorder } -type codec interface { - DecodeProviderConfig(*clusterv1.ProviderConfig, runtime.Object) error - DecodeProviderStatus(*runtime.RawExtension, runtime.Object) error - EncodeProviderStatus(runtime.Object) (*runtime.RawExtension, error) -} - // NewActuator returns a new AWS Actuator func NewActuator(params ActuatorParams) (*Actuator, error) { actuator := &Actuator{ @@ -126,7 +120,7 @@ func (a *Actuator) Create(context context.Context, cluster *clusterv1.Cluster, m } func (a *Actuator) updateMachineStatus(machine *clusterv1.Machine, awsStatus *providerconfigv1.AWSMachineProviderStatus, networkAddresses []corev1.NodeAddress) error { - awsStatusRaw, err := EncodeProviderStatus(a.codec, awsStatus) + awsStatusRaw, err := a.codec.EncodeProviderStatus(awsStatus) if err != nil { glog.Errorf("error encoding AWS provider status: %v", err) return err @@ -137,11 +131,13 @@ func (a *Actuator) updateMachineStatus(machine *clusterv1.Machine, awsStatus *pr if networkAddresses != nil { machineCopy.Status.Addresses = networkAddresses } - oldAWSStatus, err := ProviderStatusFromMachine(a.codec, machine) - if err != nil { + + oldAWSStatus := &providerconfigv1.AWSMachineProviderStatus{} + if err := a.codec.DecodeProviderStatus(machine.Status.ProviderStatus, oldAWSStatus); err != nil { glog.Errorf("error updating machine status: %v", err) return err } + // TODO(vikasc): Revisit to compare complete machine status objects if !equality.Semantic.DeepEqual(awsStatus, oldAWSStatus) || !equality.Semantic.DeepEqual(machine.Status.Addresses, machineCopy.Status.Addresses) { glog.Infof("machine status has changed, updating") @@ -164,16 +160,15 @@ func (a *Actuator) updateMachineProviderConditions(machine *clusterv1.Machine, c glog.Info("updating machine conditions") - awsStatus, err := ProviderStatusFromMachine(a.codec, machine) - if err != nil { + awsStatus := &providerconfigv1.AWSMachineProviderStatus{} + if err := a.codec.DecodeProviderStatus(machine.Status.ProviderStatus, awsStatus); err != nil { glog.Errorf("error decoding machine provider status: %v", err) return err } awsStatus.Conditions = SetAWSMachineProviderCondition(awsStatus.Conditions, conditionType, corev1.ConditionTrue, reason, msg, UpdateConditionIfReasonOrMessageChange) - err = a.updateMachineStatus(machine, awsStatus, nil) - if err != nil { + if err := a.updateMachineStatus(machine, awsStatus, nil); err != nil { return err } @@ -182,7 +177,7 @@ func (a *Actuator) updateMachineProviderConditions(machine *clusterv1.Machine, c // CreateMachine starts a new AWS instance as described by the cluster and machine resources func (a *Actuator) CreateMachine(cluster *clusterv1.Cluster, machine *clusterv1.Machine) (*ec2.Instance, error) { - machineProviderConfig, err := ProviderConfigFromMachine(a.client, machine) + machineProviderConfig, err := ProviderConfigFromMachine(a.client, machine, a.codec) if err != nil { return nil, a.handleMachineError(machine, apierrors.InvalidMachineConfiguration("error decoding MachineProviderConfig: %v", err), createEventAction) } @@ -246,7 +241,7 @@ func (a *Actuator) Delete(context context.Context, cluster *clusterv1.Cluster, m // DeleteMachine deletes an AWS instance func (a *Actuator) DeleteMachine(cluster *clusterv1.Cluster, machine *clusterv1.Machine) error { - machineProviderConfig, err := ProviderConfigFromMachine(a.client, machine) + machineProviderConfig, err := ProviderConfigFromMachine(a.client, machine, a.codec) if err != nil { return a.handleMachineError(machine, apierrors.InvalidMachineConfiguration("error decoding MachineProviderConfig: %v", err), deleteEventAction) } @@ -287,7 +282,7 @@ func (a *Actuator) DeleteMachine(cluster *clusterv1.Cluster, machine *clusterv1. func (a *Actuator) Update(context context.Context, cluster *clusterv1.Cluster, machine *clusterv1.Machine) error { glog.Info("updating machine") - machineProviderConfig, err := ProviderConfigFromMachine(a.client, machine) + machineProviderConfig, err := ProviderConfigFromMachine(a.client, machine, a.codec) if err != nil { return a.handleMachineError(machine, apierrors.InvalidMachineConfiguration("error decoding MachineProviderConfig: %v", err), noEventAction) } @@ -390,7 +385,7 @@ func (a *Actuator) Describe(cluster *clusterv1.Cluster, machine *clusterv1.Machi } func (a *Actuator) getMachineInstances(cluster *clusterv1.Cluster, machine *clusterv1.Machine) ([]*ec2.Instance, error) { - machineProviderConfig, err := ProviderConfigFromMachine(a.client, machine) + machineProviderConfig, err := ProviderConfigFromMachine(a.client, machine, a.codec) if err != nil { glog.Errorf("error decoding MachineProviderConfig: %v", err) return nil, err @@ -455,11 +450,12 @@ func (a *Actuator) updateStatus(machine *clusterv1.Machine, instance *ec2.Instan glog.Info("updating status") // Starting with a fresh status as we assume full control of it here. - awsStatus, err := ProviderStatusFromMachine(a.codec, machine) - if err != nil { + awsStatus := &providerconfigv1.AWSMachineProviderStatus{} + if err := a.codec.DecodeProviderStatus(machine.Status.ProviderStatus, awsStatus); err != nil { glog.Errorf("error decoding machine provider status: %v", err) return err } + // Save this, we need to check if it changed later. networkAddresses := []corev1.NodeAddress{} @@ -505,8 +501,7 @@ func (a *Actuator) updateStatus(machine *clusterv1.Machine, instance *ec2.Instan // awsStatus.LastELBSync = nil // } - err = a.updateMachineStatus(machine, awsStatus, networkAddresses) - if err != nil { + if err := a.updateMachineStatus(machine, awsStatus, networkAddresses); err != nil { return err } diff --git a/pkg/cloud/aws/actuators/machine/actuator_test.go b/pkg/cloud/aws/actuators/machine/actuator_test.go index 6751130973..7b9644caf4 100644 --- a/pkg/cloud/aws/actuators/machine/actuator_test.go +++ b/pkg/cloud/aws/actuators/machine/actuator_test.go @@ -239,9 +239,10 @@ func TestCreateAndDeleteMachine(t *testing.T) { if err != nil { t.Fatalf("error creating codec: %v", err) } - machineStatus, err := ProviderStatusFromMachine(codec, &updatedMachine) - if err != nil { - t.Fatalf("error getting machineStatus: %v", err) + + machineStatus := &providerconfigv1.AWSMachineProviderStatus{} + if err := codec.DecodeProviderStatus(updatedMachine.Status.ProviderStatus, machineStatus); err != nil { + t.Fatalf("error decoding machine provider status: %v", err) } if tc.createErrorExpected { diff --git a/pkg/cloud/aws/actuators/machine/utils.go b/pkg/cloud/aws/actuators/machine/utils.go index 4bd7b4bc9c..8e9d6c1dd0 100644 --- a/pkg/cloud/aws/actuators/machine/utils.go +++ b/pkg/cloud/aws/actuators/machine/utils.go @@ -19,16 +19,14 @@ package machine import ( "fmt" - "k8s.io/apimachinery/pkg/runtime" - "github.com/golang/glog" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" - "github.com/ghodss/yaml" "golang.org/x/net/context" "k8s.io/apimachinery/pkg/types" providerconfigv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1alpha1" @@ -140,7 +138,7 @@ func terminateInstances(client awsclient.Client, instances []*ec2.Instance) erro // ProviderConfigFromMachine gets the machine provider config MachineSetSpec from the // specified cluster-api MachineSpec. -func ProviderConfigFromMachine(client client.Client, machine *clusterv1.Machine) (*providerconfigv1.AWSMachineProviderConfig, error) { +func ProviderConfigFromMachine(client client.Client, machine *clusterv1.Machine, codec *providerconfigv1.AWSProviderConfigCodec) (*providerconfigv1.AWSMachineProviderConfig, error) { var providerConfig runtime.RawExtension if machine.Spec.ProviderConfig.Value == nil && machine.Spec.ProviderConfig.ValueFrom == nil { @@ -166,24 +164,12 @@ func ProviderConfigFromMachine(client client.Client, machine *clusterv1.Machine) } var config providerconfigv1.AWSMachineProviderConfig - if err := yaml.Unmarshal(providerConfig.Raw, &config); err != nil { + if err := codec.DecodeProviderConfig(&clusterv1.ProviderConfig{Value: &providerConfig}, &config); err != nil { return nil, err } return &config, nil } -// ProviderStatusFromMachine gets the machine provider status from the specified machine. -func ProviderStatusFromMachine(codec codec, m *clusterv1.Machine) (*providerconfigv1.AWSMachineProviderStatus, error) { - status := &providerconfigv1.AWSMachineProviderStatus{} - err := codec.DecodeProviderStatus(m.Status.ProviderStatus, status) - return status, err -} - -// EncodeProviderStatus encodes the machine status into RawExtension -func EncodeProviderStatus(codec codec, awsStatus *providerconfigv1.AWSMachineProviderStatus) (*runtime.RawExtension, error) { - return codec.EncodeProviderStatus(awsStatus) -} - // IsMaster returns true if the machine is part of a cluster's control plane func IsMaster(machine *clusterv1.Machine) bool { if machineType, exists := machine.ObjectMeta.Labels[providerconfigv1.MachineTypeLabel]; exists && machineType == "master" { diff --git a/pkg/cloud/aws/actuators/machine/utils_test.go b/pkg/cloud/aws/actuators/machine/utils_test.go index a64a17f748..4429477a74 100644 --- a/pkg/cloud/aws/actuators/machine/utils_test.go +++ b/pkg/cloud/aws/actuators/machine/utils_test.go @@ -1,14 +1,15 @@ package machine import ( + "reflect" + "testing" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" - "reflect" providerconfigv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1alpha1" clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "testing" ) func init() { @@ -113,7 +114,7 @@ func TestProviderConfigFromMachine(t *testing.T) { client := fake.NewFakeClient(machineClass) for _, tc := range testCases { - decodedProviderConfig, err := ProviderConfigFromMachine(client, tc.machine) + decodedProviderConfig, err := ProviderConfigFromMachine(client, tc.machine, codec) if err != nil { t.Error(err) } diff --git a/test/machines/utils.go b/test/machines/utils.go index ccfa7fabf0..c30b842b1d 100644 --- a/test/machines/utils.go +++ b/test/machines/utils.go @@ -10,7 +10,6 @@ import ( "github.com/openshift/cluster-api-actuator-pkg/pkg/e2e/framework" providerconfigv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1alpha1" - machineutils "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/actuators/machine" ) func createSecretAndWait(f *framework.Framework, secret *apiv1.Secret) { @@ -33,7 +32,8 @@ func getMachineProviderStatus(f *framework.Framework, machine *clusterv1alpha1.M codec, err := providerconfigv1.NewCodec() Expect(err).NotTo(HaveOccurred()) - machineProviderStatus, err := machineutils.ProviderStatusFromMachine(codec, machine) + machineProviderStatus := &providerconfigv1.AWSMachineProviderStatus{} + err = codec.DecodeProviderStatus(machine.Status.ProviderStatus, machineProviderStatus) Expect(err).NotTo(HaveOccurred()) return machineProviderStatus