-
Notifications
You must be signed in to change notification settings - Fork 190
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
Bug 1960559: drop APIExtensions v1beta1 #566
Conversation
@vrutkovs: This pull request references Bugzilla bug 1960559, 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
Requesting review from QA contact: In response to this:
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. |
/hold I'd prefer RBACv1beta1 PR to land first |
/hold cancel |
/retest |
@vrutkovs: This pull request references Bugzilla bug 1960559, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
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. |
/test unit Flake |
/retest Failed to setup initial cluster in upgrade test |
/test e2e-agnostic-upgrade |
/retest |
1 similar comment
/retest |
hack/test-prerequisites.go
Outdated
@@ -36,12 +36,12 @@ func main() { | |||
if err != nil { | |||
log.Fatalf("Unable to read %s: %v", path, err) | |||
} | |||
var crd v1beta1.CustomResourceDefinition | |||
var crd v1.CustomResourceDefinition | |||
if err := yaml.Unmarshal(data, &crd); err != nil { |
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.
You don't check that the API version matches? That's not going to make you happy when it doesn't.
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.
ok, test only.
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 don't think we do that in tests. In actual flow non-v1 CRDs would be treated as "unstructered".
@wking @LalatenduMohanty any advice here? I suppose we should be tracking how many manifests are being applied by unstructured handler and attempt to minimize that number. Probably we should be exporting a new metric?
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.
My long-term goal is to completely drop the unstructured handler, and fail folks in CI if they add a new manifest type that we don't support. Logging in the unstructured handler that we can look for in gathered CI assets would help get us there.
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.
if they add a new manifest type that we don't support. Logging in the unstructured handler that we can look for in gathered CI assets would help get us there.
+1
/test e2e-agnostic-upgrade |
1 similar comment
/test e2e-agnostic-upgrade |
/test unit |
/test e2e-agnostic-upgrade |
No manifests in 4.8 payload are using it
All manifests have moved to v1
/bugzilla refresh |
@vrutkovs: This pull request references Bugzilla bug 1960559, which is invalid:
Comment In response to this:
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. |
/bugzilla refresh |
@vrutkovs: This pull request references Bugzilla bug 1960559, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
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. |
/retest |
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.
/lgtm
pods should successfully create sandboxes by other
and etcdHighNumberOfLeaderChanges
are unrelated.
/override ci/prow/e2e-agnostic-upgrade
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrutkovs, wking 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 |
@wking: Overrode contexts on behalf of wking: ci/prow/e2e-agnostic-upgrade In response to this:
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. |
/override ci/prow/e2e-agnostic |
@wking: Overrode contexts on behalf of wking: ci/prow/e2e-agnostic In response to this:
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. |
This PR is unrelated to the other recent changes, so save some time and money with a manual merge to cut off the retesting. |
@vrutkovs: All pull requests linked via external trackers have merged: Bugzilla bug 1960559 has been moved to the MODIFIED state. In response to this:
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. |
@vrutkovs: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
The original APIService reconciliation landed in 662e182 (handle APIService, Service objects, 2018-09-27, openshift#26). We definitely reconcile a bunch of Service manifests (e.g. for serving operator metrics, including the CVO's own Service), but it's not clear what the use case was for APIService. DeleteAPIServicev1 landed in 0afb8a8 (Add a manifest annotation to be used for object deletion, 2020-08-17, openshift#438), but was never consumed. We dropped APIService reconciliation support in 5681a70 (Drop APIService support, 2021-06-10, openshift#566). This commit drops the unused DeleteAPIServicev1. Auditing z-stream tips from 4.3 through 4.11: $ oc adm release extract --to 4.11 quay.io/openshift-release-dev/ocp-release-nightly@sha256:080e5cf5e3e043ac0877b8f545ba2b596016f3fd76f3f457d15060603b3615e1 $ oc adm release extract --to 4.10 quay.io/openshift-release-dev/ocp-release:4.10.13-x86_64 $ oc adm release extract --to 4.9 quay.io/openshift-release-dev/ocp-release:4.9.32-x86_64 $ oc adm release extract --to 4.8 quay.io/openshift-release-dev/ocp-release:4.8.39-x86_64 $ oc adm release extract --to 4.7 quay.io/openshift-release-dev/ocp-release:4.7.50-x86_64 $ oc adm release extract --to 4.6 quay.io/openshift-release-dev/ocp-release:4.6.57-x86_64 $ oc adm release extract --to 4.5 quay.io/openshift-release-dev/ocp-release:4.5.41-x86_64 $ oc adm release extract --to 4.4 quay.io/openshift-release-dev/ocp-release:4.4.33-x86_64 $ oc adm release extract --to 4.3 quay.io/openshift-release-dev/ocp-release:4.3.40-x86_64 $ grep -r 'kind: APIService' 4.* ...no hits...
All manifests have moved to v1.
This also bumps openshift/api (and updates k8s deps to 0.21.1 to be uniform) so that all apiextensions/v1beta1 would no longer be used.