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.

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

Add all the csi env #599

Merged
merged 2 commits into from
Mar 25, 2024

Conversation

subhamkrai
Copy link

build: add all the csi image version in csv

add all the csi related images version in csv which
ocs-operator csv merger tool was changing.

build: These are auto-generated csv changes

these are auto-generated csv changes when run
make gen-csv.

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@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 Mar 22, 2024
Copy link

openshift-ci bot commented Mar 22, 2024

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

build/csv/csv-gen.sh Outdated Show resolved Hide resolved
@b-ranto
Copy link
Member

b-ranto commented Mar 22, 2024

I have tested this locally. The CSI sidecars are properly being overloaded but none of the other images are, i.e. these images still ignore the env variables:

  • ceph-csi
  • csiaddons-sidecar
  • rook-container

Weirdly, the ceph image disappeared from the relatedImages altogether. It is only available in the alm-examples section, now.

EDIT:
These are the env variables we set for these three images DS:

  • ROOK_CSI_CEPH_IMAGE
  • ROOK_CSIADDONS_IMAGE
  • ROOK_IMAGE

@subhamkrai subhamkrai force-pushed the add-all-the-csi-env branch from a98b49b to 591dab7 Compare March 22, 2024 10:07
@subhamkrai
Copy link
Author

I have tested this locally. The CSI sidecars are properly being overloaded but none of the other images are, i.e. these images still ignore the env variables:

  • ceph-csi
  • csiaddons-sidecar
  • rook-container

Weirdly, the ceph image disappeared from the relatedImages altogether. It is only available in the alm-examples section, now.

EDIT: These are the env variables we set for these three images DS:

  • ROOK_CSI_CEPH_IMAGE
  • ROOK_CSIADDONS_IMAGE
  • ROOK_IMAGE

@b-ranto I tested locally by passing the env var in the terminal and it worked may due wrong relatedImage it was not working not sure. Could you try again?

@b-ranto
Copy link
Member

b-ranto commented Mar 22, 2024

I have re-tested it and it works fine now. It turned out we were missing authetntication and that is why the three images above failed.

@subhamkrai
Copy link
Author

I have re-tested it and it works fine now. It turned out we were missing authetntication and that is why the three images above failed.

good to know it worked

@subhamkrai
Copy link
Author

once #598 is merged will mark this ready for review

add all the csi related images version in csv which
ocs-operator csv merger tool was changing.

Signed-off-by: subhamkrai <[email protected]>
@subhamkrai subhamkrai force-pushed the add-all-the-csi-env branch from 591dab7 to fb3be9a Compare March 22, 2024 11:47
@subhamkrai subhamkrai marked this pull request as ready for review March 22, 2024 11:48
@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 Mar 22, 2024
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 22, 2024
Copy link
Member

@Nikhil-Ladha Nikhil-Ladha left a comment

Choose a reason for hiding this comment

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

lgtm

@subhamkrai subhamkrai force-pushed the add-all-the-csi-env branch from fb3be9a to 183d94f Compare March 22, 2024 15:09
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 22, 2024
@subhamkrai
Copy link
Author

subhamkrai commented Mar 22, 2024

pushed without any modification to make commitlint happy

these are auto generated csv changes when run
`make gen-csv`.

Signed-off-by: subhamkrai <[email protected]>
@subhamkrai subhamkrai force-pushed the add-all-the-csi-env branch from 183d94f to 0382944 Compare March 22, 2024 15:12
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 22, 2024
Copy link

openshift-ci bot commented Mar 22, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: b-ranto, Nikhil-Ladha, sp98, subhamkrai

The full list of commands accepted by this bot can be found 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

@subhamkrai subhamkrai merged commit 5afe4a0 into red-hat-storage:master Mar 25, 2024
48 of 49 checks passed
@subhamkrai subhamkrai deleted the add-all-the-csi-env branch March 25, 2024 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants