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

csv: use ROOK_CSI_ENABLE_CEPHFS in rook operator env #803

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

parth-gr
Copy link
Member

use the value from ocs operator config by
using the calue from operator env ROOK_CSI_ENABLE_CEPHFS

refer: red-hat-storage/ocs-operator#2970

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.

use the value from ocs operator config by
using the calue from operator env ROOK_CSI_ENABLE_CEPHFS

Signed-off-by: parth-gr <[email protected]>
valueFrom:
configMapKeyRef:
key: ROOK_CSI_ENABLE_CEPHFS
name: ocs-operator-config
Copy link

Choose a reason for hiding this comment

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

It seems odd to mount the ocs operator config to the rook operator. Instead of mounting that configmap, what if we just set the env value directly to enabled it by default? Then the user can override it with the configmap that rook already checks.

                    - name: ROOK_CSI_ENABLE_CEPHFS
                      value: "true"

Copy link
Member Author

Choose a reason for hiding this comment

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

So in the case of ocs-operator deploying rook, how user automation can set the value in the rook cm?
We thought of adding it in the ocs operator configmap and then to the rook operator deployment env varibles

Copy link
Member Author

Choose a reason for hiding this comment

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

The same pattern we follow for ROOK_CURRENT_NAMESPACE_ONLY

Copy link

Choose a reason for hiding this comment

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

Ok, thanks for the discussion, I see now that this is needed to pick up the downstream settings from ocs operator, and follows the same pattern of some other settings.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2025
@parth-gr
Copy link
Member Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 23, 2025
@parth-gr
Copy link
Member Author

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 23, 2025
Copy link

openshift-ci bot commented Jan 23, 2025

@iamniting: changing LGTM is restricted to collaborators

In response to this:

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-sigs/prow repository.

Copy link

openshift-ci bot commented Jan 23, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: iamniting, parth-gr, travisn

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

@travisn travisn merged commit e04b504 into red-hat-storage:master Jan 24, 2025
50 of 53 checks passed
@parth-gr
Copy link
Member Author

/cherry-pick release-4.18

@parth-gr parth-gr deleted the update-env-cephfs branch January 24, 2025 14:09
@openshift-cherrypick-robot

@parth-gr: new pull request created: #804

In response to this:

/cherry-pick release-4.18

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-sigs/prow repository.

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