Skip to content
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

add AGE column to custom printcolumn #2960

Merged
merged 1 commit into from
Dec 22, 2022

Conversation

bavarianbidi
Copy link
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

On all CRDs where custom printcolumns are defined, the AGE column is missing. CAPI core CRDs already have the AGE column in place.

For all CRDs where custom build-colums are defined, the AGE column will be introduced.

k get azurecluster
NAME      CLUSTER   READY   REASON   AGE
marioc1   marioc1   True             3d23h

Beside the AGE column, for azureclusteridentities, the type will also be a separate column.

k get azureclusteridentities
NAME               TYPE                     AGE
cluster-identity   ManualServicePrincipal   14d

Signed-off-by: Mario Constanti [email protected]

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

* The age of all Azureresources is now printed by running `kubectl get` (e.g. `kubectl get azurecluster`)
* `kubectl get azureclusteridentity` now prints the `type` of the Azure Identity.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Dec 20, 2022
@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 20, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @bavarianbidi. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 20, 2022
@bavarianbidi bavarianbidi requested review from mboersma and removed request for devigned and CecileRobertMichon December 21, 2022 05:26
Copy link
Contributor

@mboersma mboersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 21, 2022
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 21, 2022
@bavarianbidi
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-verify

i guess it's a flaky test:

Error: accumulating resources: accumulation err='accumulating resources from 'https://github.com/kubernetes-sigs/metrics-server/releases/download/v0.5.2/components.yaml': URL is a git repository': evalsymlink failure on '/tmp/kustomize-3768561019/releases/download/v0.5.2/components.yaml' : lstat /tmp/kustomize-3768561019/releases: no such file or directory

@CecileRobertMichon
Copy link
Contributor

@bavarianbidi thanks for the PR!

Verify job is failing with:

--- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml
+++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml
@@ -320,7 +320,7 @@ spec:
       jsonPath: .spec.type
       name: Type
       type: string
-    - description: Time duration since creation of this AzureMachine
+    - description: Time duration since creation of this AzureClusterIdentity
       jsonPath: .metadata.creationTimestamp
       name: Age
       type: date
generated files are out of date, run make generate

In order to make these changes you should only have to change the files in api/ then run make generate to auto-generate the files in config/. Please let us know if you run into any trouble!

On all CRDs where custom printcolumns are defined, the AGE column is
missing. CAPI core CRDs already have the AGE column in place.

Signed-off-by: Mario Constanti <[email protected]>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 21, 2022
@bavarianbidi
Copy link
Contributor Author

bavarianbidi commented Dec 21, 2022

In order to make these changes you should only have to change the files in api/ then run make generate to auto-generate the files in config/. Please let us know if you run into any trouble!

@CecileRobertMichon 🙈 thanks for pointing this out. Forgot the make generate-manifest run this morning when i fixed the finding from the PR.

Should be fine now.

Copy link
Contributor

@mboersma mboersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 21, 2022
@mboersma
Copy link
Contributor

I verified this is working locally:

% k get cluster-api
NAME                                                            AGE
clusterresourceset.addons.cluster.x-k8s.io/crs-calico-windows   17m
clusterresourceset.addons.cluster.x-k8s.io/csi-proxy            17m

NAME                                                                            CLUSTER            AGE
kubeadmconfig.bootstrap.cluster.x-k8s.io/machinepool-4932-control-plane-mddzr   machinepool-4932   10m
kubeadmconfig.bootstrap.cluster.x-k8s.io/machinepool-4932-mp-0                  machinepool-4932   12m

NAME                                                            CLUSTER            NODENAME                               PROVIDERID                                                                                                                                                                     PHASE     AGE   VERSION
machine.cluster.x-k8s.io/machinepool-4932-control-plane-rzwzh   machinepool-4932   machinepool-4932-control-plane-8d6wf   azure:///subscriptions/e240e532-1923-4fe0-86da-28abc43fc4c7/resourceGroups/machinepool-4932/providers/Microsoft.Compute/virtualMachines/machinepool-4932-control-plane-8d6wf   Running   10m   v1.26.0

NAME                                                 CLUSTER            REPLICAS   PHASE     AGE   VERSION
machinepool.cluster.x-k8s.io/machinepool-4932-mp-0   machinepool-4932   2          Running   12m   v1.26.0

NAME                                        PHASE         AGE   VERSION
cluster.cluster.x-k8s.io/machinepool-4932   Provisioned   12m   

NAME                                                                               CLUSTER            INITIALIZED   API SERVER AVAILABLE   REPLICAS   READY   UPDATED   UNAVAILABLE   AGE   VERSION
kubeadmcontrolplane.controlplane.cluster.x-k8s.io/machinepool-4932-control-plane   machinepool-4932   true          true                   1          1       1         0             12m   v1.26.0

NAME                                                                              VERSION   READY   STATE       AGE
azuremachinepoolmachine.infrastructure.cluster.x-k8s.io/machinepool-4932-mp-0-0   v1.26.0   true    Succeeded   7m6s
azuremachinepoolmachine.infrastructure.cluster.x-k8s.io/machinepool-4932-mp-0-1   v1.26.0   true    Succeeded   7m6s

NAME                                                                     REPLICAS   READY   STATE       AGE
azuremachinepool.infrastructure.cluster.x-k8s.io/machinepool-4932-mp-0   2          true    Succeeded   12m

NAME                                                            CLUSTER            READY   REASON   AGE
azurecluster.infrastructure.cluster.x-k8s.io/machinepool-4932   machinepool-4932   True             12m

NAME                                                                                  AGE
azuremachinetemplate.infrastructure.cluster.x-k8s.io/machinepool-4932-control-plane   12m

NAME                                                                                READY   REASON   STATE       AGE
azuremachine.infrastructure.cluster.x-k8s.io/machinepool-4932-control-plane-8d6wf   True             Succeeded   10m

NAME                                                                    TYPE               AGE
azureclusteridentity.infrastructure.cluster.x-k8s.io/cluster-identity   ServicePrincipal   12m

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 22, 2022
@k8s-ci-robot
Copy link
Contributor

@bavarianbidi: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-provider-azure-e2e 838d2d8 link unknown /test pull-cluster-api-provider-azure-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@bavarianbidi
Copy link
Contributor Author

/retest-required

@k8s-ci-robot k8s-ci-robot merged commit 91eb126 into kubernetes-sigs:main Dec 22, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.7 milestone Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants