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

Make eco recognize podTemplate changes #196

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

Conversation

gottwald
Copy link

Changes

  • Allow podTemplate changes which are handled like version upgrades with a rolling re-deployment
    cycle of all etcd members.

Verification

  • Verified on a tests cluster that I could update podTemplates with new configurations such as anti-affinity, resource requests and limits. (verified with single-node and multi-node cluster)
  • Also verified that version upgrades still work as before.

Refs #109 (but solved in a more generic way)

Changes are handled like version upgrades with a rolling re-deployment
cycle of all etcd members.
if peer, reason := nextOutdatedPeer(cluster, peers); peer != nil {
if reason == outdatedVersion {
if peer.Spec.Version == cluster.Spec.Version {
log.Info("Waiting for EtcdPeer to report expected server version", "etcdpeer-name", peer.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe put a comment here to elaborate. Just something to explain that this branch occurs when we've updated the Pod specification but the etcd peer still shows as outdated.

Copy link
Author

Choose a reason for hiding this comment

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

done


const (
outdatedVersion outdatedReason = iota + 1
outdatedPodTemplate
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this using the defaulting to infer a type of outdatedReason and a value of 0?

Copy link
Author

Choose a reason for hiding this comment

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

Not sure I fully understand your question. This is just to have a constant to differentiate between the different outdated reasons. The enumeration starts with 1 on purpose so that a possible use of a default value (0) doesn't accidentally signal a specific reason where there is none.

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