From 716083f83904e6f3869f9f27750812a07eaeeb2f Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Tue, 23 Jul 2019 15:36:33 +0200 Subject: [PATCH] Set additional machine annotations/labels to get pretty machine output Set common machine fields so when a machine provisioned in AWS is listed through `oc get machines` all additional printer colums are properly populated. E.g. region, zone, instance type. During the provisioning stage: NAME STATE TYPE REGION ZONE AGE master-machine running m4.xlarge us-east-1 us-east-1a 38s After an instance got provisioned: NAME STATE TYPE REGION ZONE AGE master-machine running m4.xlarge us-east-1 us-east-1a 40s After a machine is requsted to be deleted: NAME STATE TYPE REGION ZONE AGE master-machine shutting-down m4.xlarge us-east-1 us-east-1a 24m In order to properly display values from all the annotations, one needs to update the machine CRD to have: spec: additionalPrinterColumns: - JSONPath: .metadata.annotations['machine\.openshift\.io/instance-state'] description: State of the AWS instance name: State type: string - JSONPath: .metadata.labels['machine\.openshift\.io/instance-type'] description: Type of instance name: Type type: string - JSONPath: .metadata.labels['machine\.openshift\.io/region'] description: Region associated with machine name: Region type: string - JSONPath: .metadata.labels['machine\.openshift.\.io/zone'] description: Zone associated with machine name: Zone type: string Be aware of all the dots being escaped. $ oc get machines -n openshift-machine-api NAME STATE TYPE REGION ZONE AGE jchaloup-kndjk-master-0 running m4.xlarge us-east-1 us-east-1a 16m jchaloup-kndjk-master-1 running m4.xlarge us-east-1 us-east-1b 16m jchaloup-kndjk-master-2 running m4.xlarge us-east-1 us-east-1c 16m jchaloup-kndjk-worker-us-east-1a-rz7fc running m4.large us-east-1 us-east-1a 15m jchaloup-kndjk-worker-us-east-1b-skcdp running m4.large us-east-1 us-east-1b 15m jchaloup-kndjk-worker-us-east-1c-x949n running m4.large us-east-1 us-east-1c 15m $ oc get machines -n openshift-machine-api -o wide NAME STATE TYPE REGION ZONE AGE NODE PROVIDERID jchaloup-kndjk-master-0 running m4.xlarge us-east-1 us-east-1a 16m ip-10-0-137-186.ec2.internal jchaloup-kndjk-master-1 running m4.xlarge us-east-1 us-east-1b 16m ip-10-0-144-222.ec2.internal jchaloup-kndjk-master-2 running m4.xlarge us-east-1 us-east-1c 16m ip-10-0-164-95.ec2.internal jchaloup-kndjk-worker-us-east-1a-rz7fc running m4.large us-east-1 us-east-1a 15m ip-10-0-131-249.ec2.internal aws:///us-east-1a/i-0d61da03d03bc6c51 jchaloup-kndjk-worker-us-east-1b-skcdp running m4.large us-east-1 us-east-1b 15m ip-10-0-149-207.ec2.internal aws:///us-east-1b/i-0dbe5a1764fa55848 jchaloup-kndjk-worker-us-east-1c-x949n running m4.large us-east-1 us-east-1c 15m ip-10-0-160-10.ec2.internal aws:///us-east-1c/i-09380f6badaad266e --- pkg/actuators/machine/actuator.go | 67 +++++++++++++++++++++++++++++- pkg/actuators/machine/instances.go | 3 +- pkg/actuators/machine/utils.go | 13 ++++-- 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/pkg/actuators/machine/actuator.go b/pkg/actuators/machine/actuator.go index a6f877dc97..61143ee004 100644 --- a/pkg/actuators/machine/actuator.go +++ b/pkg/actuators/machine/actuator.go @@ -42,6 +42,8 @@ import ( awsclient "sigs.k8s.io/cluster-api-provider-aws/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client" + + machinecontroller "github.com/openshift/cluster-api/pkg/controller/machine" ) const ( @@ -122,9 +124,58 @@ func (a *Actuator) Create(context context.Context, cluster *clusterv1.Cluster, m if err != nil { return fmt.Errorf("%s: failed to update machine object with providerID: %v", machine.Name, err) } + + updatedMachine, err = a.setMachineCloudProviderSpecifics(updatedMachine, instance) + if err != nil { + return fmt.Errorf("%s: failed to set machine cloud provider specifics: %v", machine.Name, err) + } + return a.updateStatus(updatedMachine, instance) } +// updateProviderID adds providerID in the machine spec +func (a *Actuator) setMachineCloudProviderSpecifics(machine *machinev1.Machine, instance *ec2.Instance) (*machinev1.Machine, error) { + if instance == nil { + return machine, nil + } + machineCopy := machine.DeepCopy() + + if machineCopy.Labels == nil { + machineCopy.Labels = make(map[string]string) + } + + if machineCopy.Annotations == nil { + machineCopy.Annotations = make(map[string]string) + } + + // Reaching to machine provider config since the region is not directly + // providing by *ec2.Instance object + machineProviderConfig, err := providerConfigFromMachine(machine, a.codec) + if err != nil { + return nil, fmt.Errorf("error decoding MachineProviderConfig: %v", err) + } + + machineCopy.Labels[machinecontroller.MachineRegionLabelName] = machineProviderConfig.Placement.Region + + if instance.Placement != nil { + machineCopy.Labels[machinecontroller.MachineAZLabelName] = aws.StringValue(instance.Placement.AvailabilityZone) + } + + if instance.InstanceType != nil { + machineCopy.Labels[machinecontroller.MachineInstanceTypeLabelName] = aws.StringValue(instance.InstanceType) + } + + if instance.State != nil && instance.State.Name != nil { + machineCopy.Annotations[machinecontroller.MachineInstanceStateAnnotationName] = aws.StringValue(instance.State.Name) + } + + if err := a.client.Update(context.Background(), machineCopy); err != nil { + return nil, fmt.Errorf("%s: error updating machine spec: %v", machine.Name, err) + } + + return machineCopy, nil +} + // updateProviderID adds providerID in the machine spec func (a *Actuator) updateProviderID(machine *machinev1.Machine, instance *ec2.Instance) (*machinev1.Machine, error) { existingProviderID := machine.Spec.ProviderID @@ -325,10 +376,19 @@ func (a *Actuator) DeleteMachine(cluster *clusterv1.Cluster, machine *machinev1. return nil } - err = terminateInstances(client, instances) + terminatingInstances, err := terminateInstances(client, instances) if err != nil { return a.handleMachineError(machine, apierrors.DeleteMachine(err.Error()), noEventAction) } + + if len(terminatingInstances) == 1 { + if terminatingInstances[0] != nil && terminatingInstances[0].CurrentState != nil && terminatingInstances[0].CurrentState.Name != nil { + machineCopy := machine.DeepCopy() + machineCopy.Annotations[machinecontroller.MachineInstanceStateAnnotationName] = aws.StringValue(terminatingInstances[0].CurrentState.Name) + a.client.Update(context.Background(), machineCopy) + } + } + a.eventRecorder.Eventf(machine, corev1.EventTypeNormal, "Deleted", "Deleted machine %v", machine.Name) return nil @@ -404,6 +464,11 @@ func (a *Actuator) Update(context context.Context, cluster *clusterv1.Cluster, m a.eventRecorder.Eventf(machine, corev1.EventTypeNormal, "Updated", "Updated machine %v", machine.Name) + machine, err = a.setMachineCloudProviderSpecifics(machine, newestInstance) + if err != nil { + return fmt.Errorf("%s: failed to set machine cloud provider specifics: %v", machine.Name, err) + } + // We do not support making changes to pre-existing instances, just update status. return a.updateStatus(machine, newestInstance) } diff --git a/pkg/actuators/machine/instances.go b/pkg/actuators/machine/instances.go index a8506a33a6..c377a72841 100644 --- a/pkg/actuators/machine/instances.go +++ b/pkg/actuators/machine/instances.go @@ -45,7 +45,8 @@ func removeStoppedMachine(machine *machinev1.Machine, client awsclient.Client) e return nil } - return terminateInstances(client, instances) + _, err = terminateInstances(client, instances) + return err } func buildEC2Filters(inputFilters []providerconfigv1.Filter) []*ec2.Filter { diff --git a/pkg/actuators/machine/utils.go b/pkg/actuators/machine/utils.go index dd07a30819..98bebcc2e1 100644 --- a/pkg/actuators/machine/utils.go +++ b/pkg/actuators/machine/utils.go @@ -144,7 +144,7 @@ func getVolume(client awsclient.Client, volumeID string) (*ec2.Volume, error) { } // terminateInstances terminates all provided instances with a single EC2 request. -func terminateInstances(client awsclient.Client, instances []*ec2.Instance) error { +func terminateInstances(client awsclient.Client, instances []*ec2.Instance) ([]*ec2.InstanceStateChange, error) { instanceIDs := []*string{} // Cleanup all older instances: for _, instance := range instances { @@ -158,12 +158,17 @@ func terminateInstances(client awsclient.Client, instances []*ec2.Instance) erro terminateInstancesRequest := &ec2.TerminateInstancesInput{ InstanceIds: instanceIDs, } - _, err := client.TerminateInstances(terminateInstancesRequest) + output, err := client.TerminateInstances(terminateInstancesRequest) if err != nil { glog.Errorf("Error terminating instances: %v", err) - return fmt.Errorf("error terminating instances: %v", err) + return nil, fmt.Errorf("error terminating instances: %v", err) } - return nil + + if output == nil { + return nil, nil + } + + return output.TerminatingInstances, nil } // providerConfigFromMachine gets the machine provider config MachineSetSpec from the