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

Fix #191 - Changing deployment strategy to recreate and setting replicas to max 1 #192

Merged
merged 4 commits into from
Dec 14, 2020

Conversation

ricardozanini
Copy link
Member

Fixes #191

@LCaparelli do you mind taking a look? Maybe improving the doc a little bit? I'm lacking creativity...

@bszeti I'll push an image with this fix soon, do you mind taking testing in your local environment? As soon as we review and test this PR, I'll release a new version with this fix for you.

Thanks!

@ricardozanini ricardozanini added this to the v0.5.0 milestone Dec 9, 2020
@ricardozanini ricardozanini self-assigned this Dec 9, 2020
Signed-off-by: Ricardo Zanini <[email protected]>
@ricardozanini
Copy link
Member Author

@LCaparelli @bszeti the image is available at: quay.io/m88i/nexus-operator:0.5.0-cr1 can be verified with the [nexus-operator.yaml file](https://github.com/m88i/nexus-operator/blob/main/nexus-operator.yaml). Just change the image tag there.

@ricardozanini
Copy link
Member Author

OLM test may fail since they changed some bits. @Kaitou786 since you've changed that on Kogito, mind replicating the fix here as well? 🤭

Copy link
Member

@LCaparelli LCaparelli left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this @ricardozanini! I didn't find the doc section lacking, it's straightforward and clear, nothing to add on my behalf but linking the HA page from Sonatype.

I do think the logic that controls spec.replica should be placed at a default setting method, not a validation one.

README.md Outdated Show resolved Hide resolved
controllers/nexus/resource/validation/validation.go Outdated Show resolved Hide resolved
@LCaparelli
Copy link
Member

@ricardozanini just tested the release candidate and things are working as expected. Setting spec.replicas to 3 changes it back to 1, it gets logged fine and any changes which cause a new ReplicaSet to be created now destroy the existing pods from the previous RS first. :-D

@ricardozanini ricardozanini added the bug 🐛 Something isn't working label Dec 10, 2020
Signed-off-by: Ricardo Zanini <[email protected]>
@ricardozanini
Copy link
Member Author

@LCaparelli can you take a final look? I've moved to defaults instead. Hopefully, with admission controllers, we wouldn't need that, since the CR won't be created.

@LCaparelli
Copy link
Member

@LCaparelli can you take a final look? I've moved to defaults instead.

This looks great man, thanks!

Hopefully, with admission controllers, we wouldn't need that, since the CR won't be created.

Well, we will, but it will be part of the mutating webhook, not part of the reconciliation logic.

@LCaparelli LCaparelli merged commit 8a3f54f into main Dec 14, 2020
@ricardozanini ricardozanini deleted the issues-191 branch December 14, 2020 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pod fails to start after modifying the Nexus resource
2 participants