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

[patch] Do not validate storage class for mongo update #1050

Closed
wants to merge 27 commits into from

Conversation

racree
Copy link
Contributor

@racree racree commented Oct 16, 2023

Storage class isn't set in the update pipeline. The mongo role just needs to not do anything with storage classes in the action=update path, update doesn't need to change the storage.

Therefore the update action should not call tasks/determine-storage-classes.yml

@racree racree changed the title [patch] do not validate storage class for update [patch] do not validate storage class for mongo update Oct 16, 2023
@racree racree changed the title [patch] do not validate storage class for mongo update [patch] Do not validate storage class for mongo update Oct 16, 2023
@racree
Copy link
Contributor Author

racree commented Oct 16, 2023

ran "mas install" specifying catalog from Aug 29, 2023

Screenshot 2023-10-16 at 1 54 58 PM

MongoDB successfully installed
Screenshot 2023-10-16 at 1 57 07 PM

Then ran "mas update" specifying catalog from Oct 4, 2023
Screenshot 2023-10-16 at 1 55 17 PM

Update pipeline successfully ran

Screenshot 2023-10-16 at 1 58 55 PM

@racree racree marked this pull request as ready for review October 16, 2023 19:00
@racree racree requested a review from rawa-resul October 16, 2023 19:00
andrercm
andrercm previously approved these changes Oct 16, 2023
Copy link
Contributor

@andrercm andrercm left a comment

Choose a reason for hiding this comment

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

makes sense to me

rawa-resul
rawa-resul previously approved these changes Oct 16, 2023
@racree racree dismissed stale reviews from rawa-resul and andrercm via 03bc87b October 17, 2023 14:23
@racree racree requested a review from whitfiea October 17, 2023 16:07
Copy link
Contributor

@whitfiea whitfiea left a comment

Choose a reason for hiding this comment

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

@racree I am not sure I understand how this fixes the problem stated, which was to not validate the storage class for mongo?

For the upgrade flow, it will call install, and if the install needs to upgrade (say from 0.7.8 to 0.7.9) then it will install this new CR https://github.com/ibm-mas/ansible-devops/blob/master/ibm/mas_devops/roles/mongodb/templates/community/0.7.9/cr.yml.j2#L75C35-L75C56 which has the mongodb storage class. So either the update needs to know the storage class or it has to read what the current storageclass is and set that in the new CR.

@racree
Copy link
Contributor Author

racree commented Nov 10, 2023

@whitfiea Does this PR need to be written differently? My original understanding was... when Mongo is already installed and running, we do not need to validate the storage class because we should not be changing the storage class. But based on your comment above, you are expecting something different?

@racree
Copy link
Contributor Author

racree commented Nov 10, 2023

@whitfiea I think I understand what you are saying... it is not enough to skip calling "tasks/determine-storage-classes" because the cr that gets applied during the update requires mongodb_storage_class to be defined.

@racree racree marked this pull request as draft November 10, 2023 17:25
@racree racree marked this pull request as ready for review November 14, 2023 20:24
@racree racree requested a review from whitfiea November 14, 2023 20:25
@racree racree marked this pull request as draft November 17, 2023 15:09
@racree
Copy link
Contributor Author

racree commented Dec 1, 2023

fixed in ibm-mas/cli#674

@racree racree closed this Dec 1, 2023
@racree racree deleted the mongodb-update branch December 1, 2023 21:34
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.

5 participants