MCO-593: Consume MCO API and CRDs from openshift/api#3747
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@jkyros: This pull request references MCO-593 which is a valid jira issue. DetailsIn 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. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
approved
|
/test e2e-gcp-op |
|
/test e2e-aws-ovn-upgrade |
jkyros
force-pushed
the
migrate-mco-api
branch
2 times, most recently
from
a97bce5 to
648de33
Compare
|
/test all |
|
/test verify |
|
/test all |
|
/test unit |
|
/test e2e-gcp-op |
1 similar comment
|
/test e2e-gcp-op |
|
/retest-required |
|
@jkyros I think you need to modify the codegen to also fetch the |
openshift-merge-robot
added
the
needs-rebase
jkyros
force-pushed
the
migrate-mco-api
branch
from
c87aa05 to
fb30e3d
Compare
openshift-merge-robot
removed
the
needs-rebase
|
/test all |
|
/test e2e-aws-ovn-upgrade |
|
Oh, hah, we're failing the Everything else is green though, so I think we've got it aside from that. |
jkyros
changed the title
|
@jkyros: This pull request references MCO-593 which is a valid jira issue. DetailsIn 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. |
jkyros
force-pushed
the
migrate-mco-api
branch
from
fb30e3d to
59a5b0d
Compare
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
@jkyros: trigger 1 job(s) for the /payload-(job|aggregate) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c8d64630-699a-11ee-9f2b-dc030e30b910-0 |
|
Overall LGTM, we will need to look at failing HyperShift conformance job |
We've been keeping the kubelet.yaml template in sync with the openshift/api featuregates manually for quite awhile. A bunch of new featuregates graduated, so we're going to do one more round, but we do need to come back and do this the right way by reading the default featuregates off the featureGateAccessor and injecing them into the kubelet.yaml templates. The plumbing is already there, we just need to use it. I also added some additional verbosity to the skew test because I got tired of having to hunt through the list for the missing gate :)
This updates our imports to read the API from openshift/api, removes the generated and API packages (because client-go supersedes them) and moves the api helper functions to their own package "apihelpers" since they did not go with the API. This also removes the "legacy" MCO api from the repo, so anyone using it will either have to pin to an older version or migreate to using it from openshift/api
There were a few API helper functions that were part of the API when it lived here, but since they were only used here in the MCO, we didn't migrate them to openshift/api, we just broke them out into a separate apihelpers package. Initially we thought we might be able to stuff them into controller/common but with the way the test suite is setup, that results in a dependency cycle, so a separate package was the way to go.
We merged some additional code since I went through and changed all the imports and references, this just makes sure the new code we've added is properly referencing the MCO API and its helpers.
We took these certificate observability fields out because they were the wrong type and put in a storageversionmigration for them in 4.14, but now that we're in 4.15, and we have a newly generated client-go for the MCO that uses applyconfigurations/server-side apply, it matters that we're not supplying the "whole object" every time -- "partially" applying them like we were as a stopgap will now generate errors and fail. This reintroduces those time fields to the MCO code so certificate observability work again, and the "required field not supplied" errors will cease.
|
Testing in #3976 it looks like it's actually the "CRDs from vendor" commit that breaks it. I might have changed the operator timing by removing the CRD sync from the operator somehow, or Hypershift cares that controllerconfig is part of the payload now for some reason? ( both possibilities are weird, but this seems like a weird thing ) If not having this PR in is blocking stuff, I can peel that CRD commit of and then we can troubleshoot that separately? |
|
As soon as this run fails, I'll peel the commit off (I just want to rule out a problem being fixed in the background): |
|
@jkyros: trigger 1 job(s) for the /payload-(job|aggregate) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/10060be0-6d5a-11ee-8230-a87660254d7b-0 |
|
@jkyros looks like it failed? |
This temporarily introduces a crds-sync.sh script that syncs the CRDs out of vendor into their usual locations. It is intended to run during "make update" and is a stopgap until we can figure out why sourcing the the manifests out of vendor and having the CVO apply controllerconfig instead of the MCO breaks hypershift external cloud provider conformance.
jkyros
force-pushed
the
migrate-mco-api
branch
from
f81c01b to
16f26e7
Compare
|
/payload-job periodic-ci-openshift-hypershift-release-4.15-periodics-e2e-aws-ovn-conformance Just another data point to see if we missed anything |
|
@yuqi-zhang: trigger 1 job(s) for the /payload-(job|aggregate) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/dbc2fd90-6ded-11ee-81fd-4ee50d4e6af2-0 |
|
cluster failure, and a pod sandbox failure |
|
/test e2e-openstack |
|
@jkyros: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
Alright, ci passed, hypershift conformance passed. I'll let this run overnight: |
|
@jkyros: trigger 8 job(s) of type blocking for the nightly release of OCP 4.15
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/51d7a4b0-6e50-11ee-99e0-c5c595fbd5a4-0 |
|
Oh wow that payload test was super green. 😄 The Azure one was the only one that failed, and it passed twice, failed twice. Failures were: which I'm reasonably confident this didn't affect. |
|
/lgtm |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdoern, jkyros, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
6 participants
This just brings:
back into the MCO.
This is almost entirely find/replace except: