Skip to content
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

Upgrade to Rook v1.5.1 #912

Merged
merged 3 commits into from
Dec 2, 2020
Merged

Upgrade to Rook v1.5.1 #912

merged 3 commits into from
Dec 2, 2020

Conversation

umangachapagain
Copy link
Contributor

Rook 1.5.1 renamed *.crd.yaml to *.crds.yaml and uses apiextensions/v1

@umangachapagain umangachapagain added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 24, 2020
@umangachapagain umangachapagain requested review from jarrpa and removed request for davidvossel November 24, 2020 07:05
@umangachapagain umangachapagain removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 24, 2020
@umangachapagain
Copy link
Contributor Author

/retest

@obnoxxx
Copy link
Contributor

obnoxxx commented Nov 24, 2020

/test ocs-operator-bundle-e2e-aws

@umangachapagain
Copy link
Contributor Author

/hold
generated CRD is not valid. Needs rework.

@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 25, 2020
@obnoxxx obnoxxx mentioned this pull request Nov 25, 2020
Merged
5 tasks
@obnoxxx
Copy link
Contributor

obnoxxx commented Nov 26, 2020

/hold
generated CRD is not valid. Needs rework.

@umangachapagain are you referring to #919 ?

@umangachapagain
Copy link
Contributor Author

/hold
generated CRD is not valid. Needs rework.

@umangachapagain are you referring to #919 ?

No. It's a different issue related to using apiextension/v1.

@obnoxxx obnoxxx added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 27, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 28, 2020
@umangachapagain
Copy link
Contributor Author

/retest

@obnoxxx obnoxxx mentioned this pull request Nov 30, 2020
Closed
@umangachapagain
Copy link
Contributor Author

/retest

@umangachapagain umangachapagain removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 30, 2020
@raghavendra-talur
Copy link
Contributor

@umangachapagain I think you need to update the rook version in go.mod and also commit changes after running make deps-update

@umangachapagain
Copy link
Contributor Author

@umangachapagain I think you need to update the rook version in go.mod and also commit changes after running make deps-update

It is done in a separate PR with all the other dependencies. It requires special handling as there are some breaking changes.

@umangachapagain
Copy link
Contributor Author

ci/prow/ocs-operator-bundle-e2e-aws fails here (waiting for collector pod) but I was able to manually deploy an OCS cluster with builds from this PR. We can override this test IMO.

@jarrpa
Copy link
Member

jarrpa commented Dec 1, 2020

Let's kick off the e2e tests one more time. I'll also validate it on my end.

/test ocs-operator-bundle-e2e-aws

@crombus crombus mentioned this pull request Dec 1, 2020
Merged
@jarrpa
Copy link
Member

jarrpa commented Dec 1, 2020

/test ocs-operator-bundle-e2e-aws
/override ci/prow/red-hat-storage-ocs-ci-e2e-aws

@openshift-ci-robot
Copy link

@jarrpa: Overrode contexts on behalf of jarrpa: ci/prow/red-hat-storage-ocs-ci-e2e-aws

In response to this:

/test ocs-operator-bundle-e2e-aws
/override ci/prow/red-hat-storage-ocs-ci-e2e-aws

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.

@jarrpa
Copy link
Member

jarrpa commented Dec 1, 2020

For me it's failing on Waiting on canary pod for node <node>. Did something change in Rook-Ceph such that the canary Pods are no longer deployed? If so we should update the deploy-manager.

@umangachapagain
Copy link
Contributor Author

/retest

@raghavendra-talur
Copy link
Contributor

For me it's failing on Waiting on canary pod for node <node>. Did something change in Rook-Ceph such that the canary Pods are no longer deployed? If so we should update the deploy-manager.

I don't fully understand the change but I think rook/rook@772865f might be the reason.

It is part of the PR rook/rook#6489 .

@raghavendra-talur
Copy link
Contributor

For me it's failing on Waiting on canary pod for node <node>. Did something change in Rook-Ceph such that the canary Pods are no longer deployed? If so we should update the deploy-manager.

I don't fully understand the change but I think rook/rook@772865f might be the reason.

It is part of the PR rook/rook#6489 .

Never mind. @jarrpa noted in a private chat that the canary pods were removed in https://github.com/rook/rook/pull/6497/files#diff-32a03f72cbf2b7d85c30ed8a2e261815b0281c9b7ccc262cf9e59556cc9fd826L116-L152

@umangachapagain
updates rook to v1.5.1
d042580 
updates rook to v1.5.1 which updated the crd filenames from
*.crd.yaml to *.crds.yaml.

Whitelisted *.crds.yaml so that it can be copied over to
operator bundle and not fail bundle validations.

Removes wait for canary pods as it is no longer deployed.

Signed-off-by: Umanga Chapagain <chapagainumanga@gmail.com>
@umangachapagain
adds support for apiextension/v1 CRD generation
7866279 
OCS Operator and Rook uses apiextensions/v1 for CRDs whereas
NooBaa Operator uses apiextensions/v1beta1. So temporarily
adding support for both while we wait for NooBaa to upgrade
to apiextensions/v1.

Signed-off-by: Umanga Chapagain <chapagainumanga@gmail.com>
@umangachapagain
updates CRDs and CSV to latest
a82bff7 
running `make gen-latest-csv` to fetch latest manifests

Signed-off-by: Umanga Chapagain <chapagainumanga@gmail.com>
@openshift-merge-robot
Copy link
Contributor

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

Test name Commit Details Rerun command
ci/prow/red-hat-storage-ocs-ci-e2e-aws a82bff7 link /test red-hat-storage-ocs-ci-e2e-aws

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.

obnoxxx
obnoxxx approved these changes Dec 2, 2020
View reviewed changes
Copy link
Contributor

@obnoxxx obnoxxx left a comment

Choose a reason for hiding this comment

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

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: obnoxxx, raghavendra-talur

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 5fdd0e4 into red-hat-storage:master Dec 2, 2020
@umangachapagain umangachapagain deleted the rook-1.5 branch December 2, 2020 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raghavendra-talur raghavendra-talur raghavendra-talur approved these changes

@obnoxxx obnoxxx obnoxxx approved these changes

@jarrpa jarrpa Awaiting requested review from jarrpa

Assignees

@obnoxxx obnoxxx

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.

None yet
7 participants
@umangachapagain @obnoxxx @raghavendra-talur @jarrpa @openshift-ci-robot @openshift-merge-robot @openshift-bot