Reflect currently running version from status.version in additionalPrinterColumns #3549
Conversation
When running for example `kubectl get elastic`, we get the version displayed from `spec.version`. It represents the desired version but not the version currently running. Now that we have `status.version` available to represent the version currently running, let's use it!
config/crds/all-crds.yaml
Outdated
| type: string | ||
| - JSONPath: .metadata.creationTimestamp | ||
| name: age | ||
| type: date |
There was a problem hiding this comment.
additionalPrinterColumns from v1beta1 does not match additionalPrinterColumns from v1 anymore with this change.
I need to double-check this does not break crd upgrade on some environments... will try the ECK 0.9 -> ECK from this PR upgrade path with Openshift 3.11 (minishift) and report here.
There was a problem hiding this comment.
Applying the new all-in-one on Openshift 3.11:
error: error validating "config/all-in-one.yaml": error validating data: [ValidationError(CustomResourceDefinition.spec.versions[0]): unknown field "additionalPrinterColumns" in io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1beta1.CustomResourceDefinitionVersion, ValidationError(CustomResourceDefinition.spec.versions[1]): unknown field "additionalPrinterColumns" in io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1beta1.CustomResourceDefinitionVersion]; if you choose to ignore these errors, turn validation off with --validate=false
😞
There was a problem hiding this comment.
I think some kustomize tweaks to ensure only the newest additionalPrinterColumns is set at the root level would fix this.
There was a problem hiding this comment.
3c57b29 sets up the required kustomize patches in order to have a single top-level additionalPrinterColumns .
If this new CRD is used with an operator running an older version (whichis unlikely to happen), or during the first few seconds before a reconciliation happens with ECK 1.3, then the version field in kubectl get elastic has an empty value, which is an OK behaviour IMO.
I can correctly apply the all-in-one manifest on Openshift 1.11 now.
Hm that's a good question. We could argue it should have been this way to start with and the previous version value was incorrect (so more like a bugfix/enhancement). @elastic/cloud-k8s any other opinion? |
👍 , sounds like a fix to me |
sebgl
changed the title
Having additionalPrinterColumns set at the `versions[]` array level in the CRD is not compatible with k8s 1.11. This commit patches the generated CRD to move the additionalPrinterColumns of the v1 version at the top-level. If this new CRD is used with an operator running an older version (which is unlikely to happen), or during the first seconds before a reconciliation happens with ECK 1.3, then the `version` field appears empty in `kubectl get elastic`.
…interColumns (elastic#3549) * Reflect version from status.version in additionalPrinterColumns When running for example `kubectl get elastic`, we get the version displayed from `spec.version`. It represents the desired version but not the version currently running. Now that we have `status.version` available to represent the version currently running, let's use it! * Use a kustomize patch to ensure a top-level additionalPrinterColumns Having additionalPrinterColumns set at the `versions[]` array level in the CRD is not compatible with k8s 1.11. This commit patches the generated CRD to move the additionalPrinterColumns of the v1 version at the top-level. If this new CRD is used with an operator running an older version (which is unlikely to happen), or during the first seconds before a reconciliation happens with ECK 1.3, then the `version` field appears empty in `kubectl get elastic`.
When running for example
kubectl get elastic, we get the versiondisplayed from
spec.version. It represents the desired version but notthe version currently running.
Now that we have
status.versionavailable to represent the versioncurrently running, let's use it instead!