Skip to content

add current profile annotations to CVO manifests #261

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.

Sign up for GitHub

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

Jump to bottom
Closed
guillaumerose wants to merge 1 commit into from
Closed

add current profile annotations to CVO manifests #261

guillaumerose wants to merge 1 commit into from

Conversation

@guillaumerose
Copy link
Contributor

@guillaumerose guillaumerose commented Oct 21, 2020

This is matches openshift/enhancements#414 and doesn't change existing behavior

@openshift-ci-robot openshift-ci-robot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Oct 21, 2020
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 21, 2020

@guillaumerose: This pull request references Bugzilla bug 1871890, which is invalid:

  • expected the bug to target the "4.7.0" release, but it targets "4.6.0" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, ON_DEV, POST, POST, but it is VERIFIED instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

bug 1871890: add current profile annotations to CVO manifests

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-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Oct 21, 2020
@guillaumerose guillaumerose mentioned this pull request Oct 21, 2020
Merged
@dgoodwin
Copy link
Contributor

dgoodwin commented Oct 21, 2020

The verify test is failing on:

 diff vendor/github.com/openshift/api/operator/v1/0000_40_cloud-credential-operator_00_config.crd.yaml ./manifests/00-config-custresdef.yaml
4a5,6
>   annotations:
>     include.release.openshift.io/self-managed-high-availability: "true"
make: *** [verify-vendored-crds] Error 1

This particular CRD definition is coming from openshift/api, and because our manifest does not match what is vendored it's complaining. It looks like you need to add the annotation to the upstream CRD in openshift/api. Unfortunately that may not be so easy as I believe it's autogenerated from go types in that repo. Frankly I think you may next hit that problem in this repo as well for the CredentialsRequest CRD definition.

It might be best to fix both by splicing the annotation in in the Makefile in update-vendored-crds, and/or update/update-codegen, I'm not totally sure when the CredReq crd file is generated in this inherited makefile structure.

@twiest twiest removed their request for review October 21, 2020 15:50
@guillaumerose guillaumerose changed the title bug 1871890: add current profile annotations to CVO manifests add current profile annotations to CVO manifests Oct 22, 2020
@openshift-ci-robot openshift-ci-robot removed the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Oct 22, 2020
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 22, 2020

@guillaumerose: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

add current profile annotations to CVO manifests

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-robot openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Oct 22, 2020
@guillaumerose guillaumerose force-pushed the master branch 2 times, most recently from 5a8b3a3 to 295999d Compare October 22, 2020 08:29
@guillaumerose
Copy link
Contributor Author

guillaumerose commented Oct 22, 2020

I updated openshift/api vendoring and run make update.
It forced me to bump k8s from 1.19 to 1.19.2.

Let see if the CI is happy now.

@guillaumerose
Copy link
Contributor Author

guillaumerose commented Oct 22, 2020

/retest

@deads2k
Copy link
Contributor

deads2k commented Oct 26, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2020
@dgoodwin
Copy link
Contributor

dgoodwin commented Oct 26, 2020

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2020
@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 26, 2020

/retest

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

8 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 26, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 26, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 26, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 26, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 27, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 27, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 27, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 27, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 7, 2020

/retest

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

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2020
@guillaumerose
Copy link
Contributor Author

guillaumerose commented Nov 7, 2020

PR rebased

@akhil-rane
Copy link
Contributor

akhil-rane commented Nov 7, 2020

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 7, 2020
@akhil-rane
Copy link
Contributor

akhil-rane commented Nov 9, 2020

@guillaumerose This fixed the CI issue but was causing a lot of failures for release builds. So had to put it on hold for now. We are investigating.

@akhil-rane
Copy link
Contributor

akhil-rane commented Nov 9, 2020

/retest

2 similar comments
@akhil-rane
Copy link
Contributor

akhil-rane commented Nov 12, 2020

/retest

@guillaumerose
Copy link
Contributor Author

guillaumerose commented Nov 12, 2020

/retest

@guillaumerose guillaumerose mentioned this pull request Nov 16, 2020
Closed
@csrwng
Copy link
Contributor

csrwng commented Nov 17, 2020

/retest

@csrwng
Copy link
Contributor

csrwng commented Nov 17, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 17, 2020
@csrwng
Copy link
Contributor

csrwng commented Nov 17, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 17, 2020
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 17, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csrwng, deads2k, dgoodwin, guillaumerose

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:

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

@guillaumerose
Copy link
Contributor Author

guillaumerose commented Nov 17, 2020

/retest

2 similar comments
@guillaumerose
Copy link
Contributor Author

guillaumerose commented Nov 17, 2020

/retest

@guillaumerose
Copy link
Contributor Author

guillaumerose commented Nov 18, 2020

/retest

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 18, 2020

/retest

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

1 similar comment
@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 18, 2020

/retest

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

@guillaumerose
Copy link
Contributor Author

guillaumerose commented Nov 18, 2020

/retest

@dgoodwin
Copy link
Contributor

dgoodwin commented Nov 18, 2020

We've seen this from time to time where a PR just can't pass CI, but if you close the PR, open an entirely new PR with the same change, it will pass almost immediately. You may be in this category. We've just had to do this with some other PRs and it worked. Suspect something persistent in the CI assigned to a PR can get out of sync or broken.

@guillaumerose
Copy link
Contributor Author

guillaumerose commented Nov 18, 2020

ok I will do this! thanks

@guillaumerose
Copy link
Contributor Author

guillaumerose commented Nov 18, 2020

Last chance for the CI, after that, I close it :)

@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Nov 18, 2020

@guillaumerose: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws 2436ce0 link /test e2e-aws

Full PR test history. Your PR dashboard.

Details

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.

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 18, 2020

/retest

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@staebler staebler Awaiting requested review from staebler

Assignees

@csrwng csrwng

@deads2k deads2k

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@guillaumerose @openshift-ci-robot @dgoodwin @deads2k @openshift-bot @akhil-rane @csrwng @openshift-merge-robot