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.

Sign up for GitHub

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

Jump to bottom

fix: Stop creating immutable resource label that breaks "tutor k8s" on Tutor version changes #533

Merged
merged 1 commit into from
Nov 25, 2021
Merged

fix: Stop creating immutable resource label that breaks "tutor k8s" on Tutor version changes #533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG-nightly.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

Note: Breaking changes between versions are indicated by "💥".

- 💥[Bugfix] No longer track the Tutor version number in resource labels (and label selectors, which breaks the update of Deployment resources), but instead do so in resource annotations.
Copy link
Contributor

Choose a reason for hiding this comment

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

As a rule of thumb, in the changelog I prefer when we describe what we are fixing instead of how. The former is of more interest to the reader. Thus, a better changelog would be something along the lines of "Fix upgrading Kubernetes deployments when upgrading from one minor tutor version to the next".
But I don't want to block your PR for a changelog line, so let's merge this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but in this case your abbreviated version doesn't contain any explanation as to why this is a breaking change. (Why should a fix to upgrades be a breaking change?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed. The changelog should also explain that the deployments should be deleted prior to upgrading.

- [Bugfix] Make it possible for plugins to implement the "caddyfile" patch without relying on the "port" local variable.
- 💥[Improvement] Move the Open edX forum to a [dedicated plugin](https://github.com/overhangio/tutor-forum/) (#450).
- 💥[Improvement] Get rid of the "tutor-openedx" package, which is no longer supported.
Expand Down
9 changes: 7 additions & 2 deletions tutor/templates/kustomization.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@ resources:
# namespace to deploy all Resources to
namespace: {{ K8S_NAMESPACE }}

# labels added to all Resources
# annotations added to all Resources
# https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/commonannotations/
commonAnnotations:
app.kubernetes.io/version: {{ TUTOR_VERSION }}

# labels (and label selectors) added to all Resources
# https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/
# https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/commonlabels/
commonLabels:
app.kubernetes.io/instance: openedx-{{ ID }}
app.kubernetes.io/version: {{ TUTOR_VERSION }}
app.kubernetes.io/part-of: openedx
app.kubernetes.io/managed-by: tutor
{{ patch("kustomization-commonlabels")|indent(2) }}
Expand Down