Skip to content

OADP-698, OADP-296 Get data mover image from manager env, migrate to RELATED_IMAGE_ #786

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
Merged
merged 4 commits into from
Aug 10, 2022
Merged

OADP-698, OADP-296 Get data mover image from manager env, migrate to RELATED_IMAGE_ #786

merged 4 commits into from
Aug 10, 2022

Conversation

kaovilai
Copy link
Member

@kaovilai kaovilai commented Aug 10, 2022 •
edited by openshift-ci bot
Loading

  • Update image getters to use new env

OADP-698
OADP-296

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 10, 2022
@openshift-ci
Copy link

openshift-ci bot commented Aug 10, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kaovilai kaovilai force-pushed the related_image-dataMoverENV branch from dd73d61 to 9d76345 Compare August 10, 2022 16:37
@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2022 •
edited
Loading

Codecov Report

Merging #786 (90c907b) into master (4c1c497) will decrease coverage by 0.16%.
The diff coverage is 63.15%.

@@            Coverage Diff             @@
##           master     #786      +/-   ##
==========================================
- Coverage   31.98%   31.82%   -0.17%     
==========================================
  Files          16       16              
  Lines        3064     3067       +3     
==========================================
- Hits          980      976       -4     
- Misses       1993     2000       +7     
  Partials       91       91
Impacted Files Coverage Δ
controllers/velero.go 48.45% <50.00%> (-0.17%) ⬇️
pkg/credentials/credentials.go 21.68% <58.33%> (-2.01%) ⬇️
controllers/datamover.go 12.99% <75.00%> (+0.45%) ⬆️
controllers/restic.go 48.71% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kaovilai kaovilai force-pushed the related_image-dataMoverENV branch from b5e7bab to 1282a95 Compare August 10, 2022 16:51
@kaovilai kaovilai changed the title Add data mover to manager's env, migrate to RELATED_IMAGE OADP-698 Add data mover to manager's env, migrate to RELATED_IMAGE Aug 10, 2022
@kaovilai kaovilai marked this pull request as ready for review August 10, 2022 16:52
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 10, 2022
@kaovilai kaovilai changed the title OADP-698 Add data mover to manager's env, migrate to RELATED_IMAGE OADP-698 Get data mover from manager env, migrate to RELATED_IMAGE Aug 10, 2022
kaovilai
kaovilai commented Aug 10, 2022
View reviewed changes
build/Dockerfile.bundle
LABEL operators.operatorframework.io.bundle.channels.v1=stable
LABEL operators.operatorframework.io.bundle.channel.default.v1=stable
LABEL operators.operatorframework.io.metrics.builder=operator-sdk-v1.14.0
LABEL operators.operatorframework.io.metrics.builder=operator-sdk-v1.21.0
Copy link
Member Author

Choose a reason for hiding this comment

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

label is updated here because RELATED_IMAGE_ discovery were a recent operator-sdk change.
This tell devs to bump their operator-sdk version if they encounter related images generation issues.

We would no longer encounter issue that is supposed to be fixed by operator-framework/operator-sdk#5765 after this change.

@kaovilai kaovilai changed the title OADP-698 Get data mover from manager env, migrate to RELATED_IMAGE OADP-698 Get data mover from manager env, migrate to RELATED_IMAGE_ Aug 10, 2022
kaovilai
kaovilai commented Aug 10, 2022
View reviewed changes
controllers/datamover.go Outdated
Comment on lines 200 to 203
if os.Getenv("RELATED_IMAGE_volume-snapshot-mover") == "" {
return common.DataMoverImage
}
return os.Getenv("RELATED_IMAGE_volume-snapshot-mover")
Copy link
Member Author

Choose a reason for hiding this comment

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

This allows manager environment variable to override the image

@kaovilai kaovilai changed the title OADP-698 Get data mover from manager env, migrate to RELATED_IMAGE_ OADP-698 Get data mover image from manager env, migrate to RELATED_IMAGE_ Aug 10, 2022
rayfordj
rayfordj requested changes Aug 10, 2022
View reviewed changes
rayfordj
rayfordj approved these changes Aug 10, 2022
View reviewed changes
Copy link
Contributor

@rayfordj rayfordj 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 openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2022
@kaovilai kaovilai changed the title OADP-698 Get data mover image from manager env, migrate to RELATED_IMAGE_ OADP-698, OADP-296 Get data mover image from manager env, migrate to RELATED_IMAGE_ Aug 10, 2022
@kaovilai
Copy link
Member Author

/retest

1 similar comment
@kaovilai
Copy link
Member Author

/retest

@kaovilai
Copy link
Member Author

/test all

@kaovilai
Copy link
Member Author

/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest
/retest

@openshift-ci
Copy link

openshift-ci bot commented Aug 10, 2022

@kaovilai: all tests passed!

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.

@kaovilai kaovilai merged commit 4ea4891 into openshift:master Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rayfordj rayfordj rayfordj approved these changes

@dymurray dymurray dymurray approved these changes

@shubham-pampattiwar shubham-pampattiwar shubham-pampattiwar approved these changes

@savitharaghunathan savitharaghunathan Awaiting requested review from savitharaghunathan

@weshayutin weshayutin Awaiting requested review from weshayutin

Assignees

@rayfordj rayfordj

Labels

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.

5 participants

@kaovilai @codecov-commenter @rayfordj @dymurray @shubham-pampattiwar