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

[Helm] separate the worker and web selectors #5907

Merged
merged 2 commits into from
Nov 3, 2022
Merged

Conversation

orangewolf
Copy link
Member

Currently the selector labels for both web deployment and worker deployment are the same. This leads to the service seeing both web and worker pods. It should just see the web pods. Also it causes some UIs to be confused about how many pods there are (looking at you Rancher UI). This isolates the two deployments.

I'd like to see a chart release pushed once this is merged. I don't see any charts in the registry since 1.0.0 and we're on 1.6.0 in the code base. I can do the push if that's ok with reviewer.

@samvera/hyrax-code-reviewers

…identally get drafted in to the service for displaying pages
@mcritchlow
Copy link
Contributor

mcritchlow commented Nov 1, 2022

This seems like a great idea.

Do you want to add a chart version bump to this PR? Happy to approve/merge this.

fwiw, the charts have been published: https://github.com/orgs/samvera/packages/container/package/charts%2Fhyrax (though still in a manual fashion which is less than great)

@orangewolf
Copy link
Member Author

I was 100% looking in the wrong place! I'll bump the version up

@orangewolf
Copy link
Member Author

@mcritchlow I bumped this to 2.0.0. I'm curious what your opinion is on it. We rolled it out to an existing cluster today (I'd only used it on a new cluster before) and found that you have to remove the worker deployment before deploying this version. It respawns no problemo, but it does require that manual step. This is because the field we're changing is not mutable.

@mcritchlow
Copy link
Contributor

@orangewolf - Yeah that makes sense to me. Not to make more work (we could always start doing this later), but i wonder if we should have a section in the chart readme on Upgrading, similar to the bitnami charts https://github.com/bitnami/charts/tree/main/bitnami/nginx#upgrading

Either way, approving this 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes-container Release Notes: Docker, Helm, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants