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

controllers: scale down ocs-client-op csv in non-provider mode #474

Closed
wants to merge 1 commit into from

Conversation

leelavg
Copy link
Contributor

@leelavg leelavg commented Sep 12, 2024

t0: odf-op w/ X.Y.Z is installed and all dependecies at same version will be installed
t1: odf-op is upgraded from X.Y.Z to X.(Y+1).0 and update will happen for odf-op unless there are explicit not upgradeable conditions set
t2: dependencies will first get updated from X.Y.Z to X.Y.(Z+1) if the channel has the update, now if one of the dependencies is stuck odf-op keeps trying to check for upgraded install plan which will not be created due to previous install plan for dependencies not being fulfilled.

now, odf-op always tries to scale down client-op if not in provider mode in above scenario t1 & t2 aren't strictly linear and odf-op ended up scaling down X.Y.Z version of client-op but not X.Y.(Z+1) which will never reach running state if cluster is configured as non-provider mode.

current PR makes sure that client-op csv & deployments are always scaled down for all non-provider cases.

t0: odf-op w/ X.Y.Z is installed and all dependecies at same version
will be installed
t1: odf-op is upgraded from X.Y.Z to X.(Y+1).0 and update will happen
for odf-op unless there are explicit not upgradeable conditions set
t2: dependencies will first get updated from X.Y.Z to X.Y.(Z+1) if
the channel has the update, now if one of the dependencies is stuck
odf-op keeps trying to check for upgraded install plan which will not be
created due to previous install plan for dependencies not being
fulfilled.

now, odf-op always tries to scale down client-op if not in provider mode
in above scenario t1 & t2 aren't strictly linear and odf-op ended up
scaling down X.Y.Z version of client-op but not X.Y.(Z+1) which will
never reach running state if cluster is configured as non-provider mode.

current PR makes sure that client-op csv & deployments are always scaled
down for all non-provider cases.

Signed-off-by: Leela Venkaiah G <lgangava@ibm.com>
Copy link
Contributor

openshift-ci bot commented Sep 12, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: leelavg
Once this PR has been reviewed and has the lgtm label, please assign agarwal-mudit for approval. For more information see the Kubernetes Code Review Process.

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

@leelavg
Copy link
Contributor Author

leelavg commented Sep 12, 2024

fix for https://bugzilla.redhat.com/show_bug.cgi?id=2311857, I tested wrt expectation and scenarios that I could think of, pls review and surface any left out paths orrefactoring efforts. thanks.

Copy link
Member

@iamniting iamniting left a comment

Choose a reason for hiding this comment

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

Hi Leela, Earlier we decided that we will scale it down via the CSV. What is a need of doing it via the deployment now. With this fix we will be scaling down from both the places. IMO we should scale down there itself instead of having a new code.

Here is a PR https://github.com/red-hat-storage/odf-operator/pull/450/files

@leelavg
Copy link
Contributor Author

leelavg commented Sep 13, 2024

What is a need of doing it via the deployment now.

  • I explained the reasoning of why deployment is being brought down in code comment and why only setting single CSV replicas to 0 isn't working in PR description. May I know what more info is required?

@iamniting
Copy link
Member

We had a discussion over a call and we decided to close the BZ https://bugzilla.redhat.com/show_bug.cgi?id=2311857#c7

@iamniting iamniting closed this Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants