-
Notifications
You must be signed in to change notification settings - Fork 85
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
Improvement/zenko 4730 mongodb v6.0 #2028
base: development/2.9
Are you sure you want to change the base?
Improvement/zenko 4730 mongodb v6.0 #2028
Conversation
Hello benzekrimaha,My role is to assist you with the merge of this Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
solution-base/mongodb/patches/mongodb-sharded-add-configsvr-service.patch
Outdated
Show resolved
Hide resolved
97e1c6b
to
59b5557
Compare
@@ -46,13 +46,13 @@ images: | |||
newTag: 0.34.0-debian-11-r24 | |||
- name: metalk8s-registry-from-config.invalid/zenko-base-2.3.11/mongodb-sharded | |||
newName: docker.io/bitnami/mongodb-sharded | |||
newTag: 5.0.18-debian-11-r15 | |||
newTag: 6.0.10-debian-11-r8 | |||
- name: metalk8s-registry-from-config.invalid/zenko-base-2.3.18/bitnami-shell | |||
newName: docker.io/bitnami/bitnami-shell | |||
newTag: 10-debian-10-r404 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- bitnami shell changed as well
- it would be better to automatically "pick up" these values from solution-base, somehow...
@@ -0,0 +1,56 @@ | |||
#!/bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- these scripts are named "-sharded" : so we can put them in the same folder as the other scripts
- there is lot of redundancy here between the sharded and non-sharded scripts:
- for create-app-user, the diff seems to only be the
mongosh
(vsmongo
) command --> we should align (check version to use either mongo or mongosh? find from some other env variable? automatically 'detect' which one is available? ...) - for set-default-write-concern, it does not depend on sharding but on the version of mongo... and furtunately we don't use it in non-sharded mongo deployment
- for create-app-user, the diff seems to only be the
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=> Done here : c54d619#diff-f3e6344f5181df0001c53dac90d9291cb2ff6105589ab8a3d277e4f379f3cc5bR3
Keeping only one file and switching commands
5. Manually update the patches by applying the changes manually in the new upgraded charts (note that this can be applied only to patches that target mongo sharded) => `git diff -- solution-base/mongodb/charts/mongodb-sharded/values.yaml > solution-base/mongodb/patches/secret-name.patch` (this operation needs to be done for every patch). | ||
6. Once the patches updated apply them to the charts with the command `make patch` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to ensure this step is not missed, we should add a pre-merge step where we run step 4 (make fetch-mongodb-sharded
), and check the resulting git diff is empty:
--> this will ensure that every modification we perform on the charts is tracked in the patches as well, giving us confidence in future upgrades
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c6f0a35
to
5dad620
Compare
501f9dc
to
4d3b158
Compare
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
What does this PR do, and why do we need it?
Which issue does this PR fix?
fixes #
Special notes for your reviewers: