Move manifests to simplify codegen #555
Conversation
We spent some time in openshift#511 and again in openshift#542 trying to reconcile CRDs. The problem is that we want to *generate* the CredentialsRequest CRD from code in this repo, but *use* (copy) the CloudCredential CRD from openshift/api, which we vendor. But we invoke controller-gen through build-machinery-go, and it does unexpected things to the latter, which breaks validation. With this commit, we move the CredentialsRequest CRD to a `generated` subdirectory and the CloudCredential CRD to an `imported` subdirectory. This lets us go back to the simpler invocation of bmg's tooling while keeping everything in the shape we expect. One more quirk: Because build-machinery-go starts defining dependency chains for targets like `update`, we need to start defining that dependency chain *before* we import the bmg libs to ensure that we copy/generate CRDs *before* we include them in bindata.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 2uasimojo 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 |
openshift-ci
bot
added
the
approved
|
/assign @abutcher |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #555 +/- ##
=======================================
Coverage 48.06% 48.06%
=======================================
Files 93 93
Lines 11305 11305
=======================================
Hits 5434 5434
Misses 5251 5251
Partials 620 620
|
|
/retest |
|
More context:
If we can just get that upstream code downstreamed & released, we can simply remove all hackery around this entirely. |
|
Spoke with David and I'm going to take a stab at getting the OpenShift controller-tools fork updated so we can just bump the version we use to latest and not have to do any of this. |
|
Sounds good @stevekuznetsov. We'll want to:
We also may still need to do something akin to the |
|
@2uasimojo I chatted with the folks doing there, and it seems like the real flow will be:
Looks like they don't want to be pushing releases anymore, so the |
|
@2uasimojo also good point on bindata - I noticed the same. |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. 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. |
|
@2uasimojo: 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. |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
5 participants
We spent some time in #511 and again in #542 trying to reconcile CRDs. The problem is that we want to generate the CredentialsRequest CRD from code in this repo, but use (copy) the CloudCredential CRD from openshift/api, which we vendor. But we invoke controller-gen through build-machinery-go, and it does unexpected things to the latter, which breaks validation.
With this commit, we move the CredentialsRequest CRD to a
generatedsubdirectory and the CloudCredential CRD to animportedsubdirectory. This lets us go back to the simpler invocation of bmg's tooling while keeping everything in the shape we expect.One more quirk: Because build-machinery-go starts defining dependency chains for targets like
update, we need to start defining that dependency chain before we import the bmg libs to ensure that we copy/generate CRDs before we include them in bindata.