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

fix: make semverCompare handle GKE versions and release 2.16.1 #735

Merged
merged 1 commit into from Feb 6, 2023

Conversation

czeslavo
Copy link
Contributor

@czeslavo czeslavo commented Feb 6, 2023

What this PR does / why we need it:

Fixes semverCompare call that decides whether to create a projected volumeMount for the service account. It didn't work on GKE where the expected version format is vX.Y.Z-gke.N. It was resulting in:

  • using a reference to secret in the deployment volume (deployment.yaml)
  • not creating a secret (secret-sa-token.yaml)

which made deployment fail to become ready because of the missing secret to be mounted.

Regression was introduced in #722.

See related upstream issues:

Which issue this PR fixes

Fixes bug discovered in KTF CI run: https://github.com/Kong/kubernetes-testing-framework/actions/runs/4102865692/jobs/7076352608

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • PR is based off the current tip of the main branch.
  • Changes are documented under the "Unreleased" header in CHANGELOG.md
  • [ ] New or modified sections of values.yaml are documented in the README.md
  • Commits follow the Kong commit message guidelines

@czeslavo czeslavo self-assigned this Feb 6, 2023
@czeslavo czeslavo added the bug Something isn't working label Feb 6, 2023
@czeslavo czeslavo force-pushed the fix-projected-sa-gke branch 3 times, most recently from 225d34e to 3befff7 Compare February 6, 2023 16:32
@czeslavo czeslavo marked this pull request as ready for review February 6, 2023 16:32
@czeslavo czeslavo requested a review from a team as a code owner February 6, 2023 16:32
pmalek
pmalek previously approved these changes Feb 6, 2023
Copy link
Member

@pmalek pmalek left a comment

Choose a reason for hiding this comment

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

Nice. Don't we test on gke to detect such things?

@czeslavo czeslavo changed the title fix: make semverCompare handle GKE versions fix: make semverCompare handle GKE versions and release 2.16.1 Feb 6, 2023
@czeslavo
Copy link
Contributor Author

czeslavo commented Feb 6, 2023

Nice. Don't we test on gke to detect such things?

Nah, seems like charts are tested only against KinD, and in KIC we don't use the chart to install the controller (only proxy).

@czeslavo czeslavo merged commit cf4be8e into main Feb 6, 2023
@czeslavo czeslavo deleted the fix-projected-sa-gke branch February 6, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants