-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set additional machine annotations/labels to get pretty machine output #242
Set additional machine annotations/labels to get pretty machine output #242
Conversation
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
return nil, fmt.Errorf("error decoding MachineProviderConfig: %v", err) | ||
} | ||
|
||
machineCopy.Labels[machinecontroller.MachineRegionLabelName] = machineProviderConfig.Placement.Region |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These labels should live in the pkg/api section of cluster-api.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think region output is redundant, everything is in the same region for a given cluster. Also, could we add nodeRef? That would be nice to have.
We already have that info, no need to add anything at the actuators level https://github.com/openshift/machine-api-operator/pull/360/files#diff-ac21ceeaaa254b32670bdb3ced62278cR30 |
/retest |
In case a user switches between contexts, it's not so clear. Also, the point is to have that information right way without any additional command run. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
/test e2e-aws-operator |
… comment The updateProviderID comment was probably a copy/paste error when setMachineCloudProviderSpecifics landed in 716083f (Set additional machine annotations/labels to get pretty machine output, 2019-07-23, openshift#242). I haven't read through the function carefully enough to be able to propose a comment that adds much beyond the function name, which is fairly clear on its own. But I know the updateProviderID comment is not a good fit ;).
… comment The updateProviderID comment was probably a copy/paste error when setMachineCloudProviderSpecifics landed in 716083f (Set additional machine annotations/labels to get pretty machine output, 2019-07-23, openshift#242). I haven't read through the function carefully enough to be able to propose a comment that adds much beyond the function name, which is fairly clear on its own. But I know the updateProviderID comment is not a good fit ;).
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:
After an instance got provisioned:
After a machine is requsted to be deleted:
In order to properly display values from all the annotations,
one needs to update the machine CRD to have:
Be aware of all the dots being escaped.