Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

feat: chart: add labels to QuarksStatefulSets #1455

Merged
merged 1 commit into from
Oct 8, 2020
Merged

Conversation

mook-as
Copy link
Contributor

@mook-as mook-as commented Oct 6, 2020

Description

Add the standard labels to the generated QuarksStatefulSets, so that we can see the version when examining a live deployment easier. The selector are unaffected, as we want to be able to use the previous version for a bit during an upgrade.

Motivation and Context

When examining an upgrade, it is difficult to figure out what version of KubeCF corresponds to each pod.

How Has This Been Tested?

Deployed during an upgrade (HA). I have confirmed that:

  • The StatefulSets are updated, not recreated (their creation timestamp is from before the upgrade took place).
  • The existing pods are rolling as expected — api-1 is upgraded and ready before api-0 is upgraded.

(Actually, the upgrade never completes because api-1 is not counted for bootstrap, so DB migration never takes place, and therefore api-1 can never get ready. But that's an existing issue outside this PR.)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code has security implications.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Add the standard labels to the generated QuarksStatefulSets, so that we
can see the version when examining a live deployment easier.  The
selector are unaffected, as we want to be able to use the previous
version for a bit during an upgrade.
@mook-as mook-as requested a review from viovanov October 6, 2020 23:14
@f0rmiga
Copy link
Member

f0rmiga commented Oct 7, 2020

@mook-as Can this be an annotation rather than a label?

@mook-as
Copy link
Contributor Author

mook-as commented Oct 7, 2020

@f0rmiga Sure, but the actual labels are all pretty standard. It's not really a problem to move them, but it seems odd to put them under annotations unlike every other resource.

@f0rmiga
Copy link
Member

f0rmiga commented Oct 8, 2020

@f0rmiga Sure, but the actual labels are all pretty standard. It's not really a problem to move them, but it seems odd to put them under annotations unlike every other resource.

I see. Indeed we have the rest as labels.

@jandubois jandubois merged commit 818229d into master Oct 8, 2020
@jandubois jandubois deleted the marky/qsts-labels branch October 8, 2020 03:34
@fargozhu fargozhu added this to the 2.5.7 milestone Oct 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants