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

feat: change Service selector to 'short' labels collection #108

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tomasz-spoton
Copy link

@tomasz-spoton tomasz-spoton commented Nov 7, 2024

Rationale:
since there's no way to pin monochart's version (there's no history), deployments of services that line up with new monochart version are potentially unstable because of Service's failure to bind endpoints to pods.

@tomasz-spoton tomasz-spoton requested a review from a team as a code owner November 7, 2024 10:57
@mrballcb
Copy link

mrballcb commented Nov 7, 2024

Any apps which have been deployed using this will have to be redeployed. To make it non-disruptive, it will have to be a manual series of steps. Is there a list of how many apps have been deployed with this version of the monochart?

Request: Please bump the minor version of monochart in Chart.yaml to reflect that this is a breaking change.

@tomasz-spoton
Copy link
Author

tomasz-spoton commented Nov 8, 2024

@mrballcb

  1. are you saying that monochart that is currently pulled reflects the change from this PR?
  2. I understand the ask to bump the version, but I am not sure that this is a breaking change - the matching is less restrictive, service resource is mutable and my understanding is that everything is going to be working as prior to the change with the added benefit of not being susceptible to breaking when app's deployment correlates in time with monochart version bump.

@tomasz-spoton
Copy link
Author

@mrballcb
could you elaborate? I'm fine with closing this PR if it's disruptive but I'd like to understand what you meant

@mrballcb
Copy link

@dalefrancum Can you provide some colour here as to why changing the selector is going to be a problem, or to me why it isn't a problem in this particular case?

@tomasz-spoton
Copy link
Author

tomasz-spoton commented Nov 19, 2024

@mrballcb @dalefrancum
what are the next steps here? Is this acceptable? If not, do you have an alternative solution for me or are you perhaps working on one?

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