Skip to content

Conversation

@sinnykumari
Copy link
Contributor

@sinnykumari sinnykumari commented May 7, 2021

apiextensions.k8s.io/v1beta1 will be deprecated in kube 1.22+
Updated CRDs and any usage inside MCO from v1beta to v1

Deprecated CRD fields in v1 are at https://kubernetes.io/docs/reference/using-api/deprecation-guide/#customresourcedefinition-v122

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 7, 2021
@sinnykumari sinnykumari changed the title Switch CRDs to v1 Bug 1947791: Switch CRDs to v1 May 7, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 7, 2021

@sinnykumari: This pull request references Bugzilla bug 1947791, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @mike-nguyen

Details

In response to this:

Bug 1947791: Switch CRDs to v1

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.

@openshift-ci openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels May 7, 2021
@openshift-ci openshift-ci bot requested a review from mike-nguyen May 7, 2021 15:36
@sinnykumari
Copy link
Contributor Author

@sttts @rphillips Please review this, thanks!

@sinnykumari sinnykumari requested review from rphillips and sttts May 7, 2021 15:40
@sinnykumari
Copy link
Contributor Author

Verified with spinning up a local cluster with this PR and seems like deprecated APIs related alerts from MCO has been resolved, verified as per bug comment. Also, no more warning messages apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+; use apiextensions.k8s.io/v1 CustomResourceDefinition in machine-config-operator pod log


$ masters=$(oc get no -l node-role.kubernetes.io/master | sed '1d' | awk '{print $1}')
$ oc adm node-logs $masters --path=kube-apiserver/audit.log --raw | grep -e '"k8s.io/removed-release":"1.22"' > dep.json
$ cat dep.json | jq -r '.user.username+": "+.requestURI' | sort | uniq 
system:kube-controller-manager: /apis/extensions/v1beta1/ingresses?allowWatchBookmarks=true&resourceVersion=12701&timeout=8m44s&timeoutSeconds=524&watch=true
system:kube-controller-manager: /apis/extensions/v1beta1/ingresses?allowWatchBookmarks=true&resourceVersion=12734&timeout=9m55s&timeoutSeconds=595&watch=true
system:kube-controller-manager: /apis/extensions/v1beta1/ingresses?allowWatchBookmarks=true&resourceVersion=25360&timeout=8m36s&timeoutSeconds=516&watch=true
system:kube-controller-manager: /apis/extensions/v1beta1/ingresses?allowWatchBookmarks=true&resourceVersion=25392&timeout=8m38s&timeoutSeconds=518&watch=true
system:kube-controller-manager: /apis/extensions/v1beta1/ingresses?allowWatchBookmarks=true&resourceVersion=41181&timeout=7m27s&timeoutSeconds=447&watch=true
system:kube-controller-manager: /apis/extensions/v1beta1/ingresses?allowWatchBookmarks=true&resourceVersion=43916&timeout=6m45s&timeoutSeconds=405&watch=true
system:kube-controller-manager: /apis/extensions/v1beta1/ingresses?allowWatchBookmarks=true&resourceVersion=45316&timeout=9m9s&timeoutSeconds=549&watch=true
system:kube-controller-manager: /apis/extensions/v1beta1/ingresses?allowWatchBookmarks=true&resourceVersion=46530&timeout=6m18s&timeoutSeconds=378&watch=true
system:kube-controller-manager: /apis/extensions/v1beta1/ingresses?limit=500&resourceVersion=0
system:serviceaccount:openshift-cluster-version:default: /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/clusteroperators.config.openshift.io
system:serviceaccount:openshift-cluster-version:default: /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/clusterversions.config.openshift.io
system:serviceaccount:openshift-cluster-version:default: /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/credentialsrequests.cloudcredential.openshift.io
system:serviceaccount:openshift-cluster-version:default: /apis/rbac.authorization.k8s.io/v1beta1/namespaces/openshift-machine-api/rolebindings/cluster-autoscaler-operator
system:serviceaccount:openshift-cluster-version:default: /apis/rbac.authorization.k8s.io/v1beta1/namespaces/openshift-machine-api/roles/cluster-autoscaler-operator
system:serviceaccount:openshift-machine-api:cluster-autoscaler-operator: /apis/admissionregistration.k8s.io/v1beta1/validatingwebhookconfigurations?allowWatchBookmarks=true&resourceVersion=40549&timeoutSeconds=495&watch=true
system:serviceaccount:openshift-machine-api:cluster-autoscaler-operator: /apis/admissionregistration.k8s.io/v1beta1/validatingwebhookconfigurations?allowWatchBookmarks=true&resourceVersion=43703&timeoutSeconds=410&watch=true
system:serviceaccount:openshift-machine-api:cluster-autoscaler-operator: /apis/admissionregistration.k8s.io/v1beta1/validatingwebhookconfigurations?allowWatchBookmarks=true&resourceVersion=46147&timeoutSeconds=451&watch=

/retest

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: containerruntimeconfigs.machineconfiguration.openshift.io
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have this in openshift/api ?

Copy link
Contributor

Choose a reason for hiding this comment

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

same question as for those below. All *.openshift.io APIs belong into that repo. Otherwise, we are unable to do cross-component API tasks in a consistent way and have no API review either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't think so. Doing quick grep of "containerruntimeconfigs" in openshift/api repo gives no result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree and while working on this PR I was thinking of doing a spike in 4.9 and work on moving MCO APIs to openshift/api repo.

@sinnykumari
Copy link
Contributor Author

/retest

@sinnykumari
Copy link
Contributor Author

MCO logs looks fine for e2e-agnostic-upgrade test,
/test e2e-agnostic-upgrade

Copy link
Contributor

@yuqi-zhang yuqi-zhang left a comment

Choose a reason for hiding this comment

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

First pass looks good! I don't see any references remaining to beta APIs, and it looks like the MCO is successful.

Just to make sure of something, because the diffs look somewhat funky, the actual API+verification we do shouldn't change behaviour right? (I see you manually generated some of our subfields like we did before)

@sinnykumari
Copy link
Contributor Author

Just to make sure of something, because the diffs look somewhat funky, the actual API+verification we do shouldn't change behaviour right? (I see you manually generated some of our subfields like we did before)

Yeah, it shouldn't change any behavior if corresponding behavior hasn't changed in upstream.

@sinnykumari
Copy link
Contributor Author

/retest

@sinnykumari
Copy link
Contributor Author

/test e2e-gcp-upgrade

@sinnykumari
Copy link
Contributor Author

/retest

@rphillips
Copy link
Contributor

lgtm

@sinnykumari
Copy link
Contributor Author

One more try
/test e2e-agnostic-upgrade

@sinnykumari
Copy link
Contributor Author

/test e2e-agnostic-upgrade

@sinnykumari
Copy link
Contributor Author

/retest

@yuqi-zhang
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 12, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 12, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sinnykumari, yuqi-zhang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [sinnykumari,yuqi-zhang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit ec3c68e into openshift:master May 12, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 12, 2021

@sinnykumari: All pull requests linked via external trackers have merged:

Bugzilla bug 1947791 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1947791: Switch CRDs to v1

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.

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. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants