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(konnect): allow configuring license polling periods #4178

Merged
merged 3 commits into from
Jun 15, 2023

Conversation

czeslavo
Copy link
Contributor

@czeslavo czeslavo commented Jun 14, 2023

What this PR does / why we need it:

Adds --konnect-initial-license-polling-period and --konnect-license-polling-period CLI flags that allow configuring periods at which we poll license from Konnect.

Changes the license.Agent's behavior so that if it fails to fetch a license at the first attempt, it will retry with the initial period (which by default is 1m) instead of the regular period (which is 12h). It will make it possible to perform smoother upgrades from free to higher Konnect tiers with no KIC restart required.

Which issue this PR fixes:

Closes #4147.

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@czeslavo czeslavo added backport release/2.10.x area/feature New feature or request area/konnect Issues and PRs related to Konnect labels Jun 14, 2023
@czeslavo czeslavo self-assigned this Jun 14, 2023
@czeslavo czeslavo marked this pull request as ready for review June 14, 2023 14:42
@czeslavo czeslavo requested a review from a team as a code owner June 14, 2023 14:42
@czeslavo czeslavo added the ci/run-e2e Trigger e2e test run from PR label Jun 14, 2023
@team-k8s-bot
Copy link
Collaborator

E2E (targeted) tests were started at https://github.com/Kong/kubernetes-ingress-controller/actions/runs/5268606896

@team-k8s-bot team-k8s-bot removed the ci/run-e2e Trigger e2e test run from PR label Jun 14, 2023
@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Patch coverage: 91.9% and project coverage change: +0.4 🎉

Comparison is base (c780e7f) 60.5% compared to head (ea5bbd6) 61.0%.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #4178     +/-   ##
=======================================
+ Coverage   60.5%   61.0%   +0.4%     
=======================================
  Files        150     150             
  Lines      16822   16897     +75     
=======================================
+ Hits       10194   10320    +126     
+ Misses      5984    5931     -53     
- Partials     644     646      +2     
Impacted Files Coverage Δ
internal/adminapi/konnect.go 0.0% <ø> (ø)
internal/manager/run.go 50.0% <0.0%> (-1.0%) ⬇️
internal/license/agent.go 89.7% <94.3%> (+26.8%) ⬆️
internal/dataplane/parser/parser.go 76.4% <100.0%> (+0.2%) ⬆️
internal/konnect/license/client.go 82.9% <100.0%> (+82.9%) ⬆️
internal/manager/config.go 94.9% <100.0%> (+<0.1%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@team-k8s-bot team-k8s-bot temporarily deployed to gcloud June 14, 2023 14:48 — with GitHub Actions Inactive
@mflendrich mflendrich requested a review from rainest June 14, 2023 16:17
Copy link
Contributor

@rainest rainest left a comment

Choose a reason for hiding this comment

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

The upstream endpoint apparently does weird things that it kinda shouldn't re 404s, but it looks like it was already kinda doing that. It's not clear whether the existing 404 case should have ever occurred in practice, but it was in the code, so 🤷

Main change request is to treat that as not an error, since it's expected, so we shouldn't make that the odd error out that's actually control flow. Rest is mostly comment/log clarification.

internal/license/agent.go Outdated Show resolved Hide resolved
internal/license/agent.go Outdated Show resolved Hide resolved
internal/license/agent.go Outdated Show resolved Hide resolved
internal/license/agent.go Outdated Show resolved Hide resolved
internal/license/agent.go Outdated Show resolved Hide resolved
internal/konnect/license/client.go Outdated Show resolved Hide resolved
internal/license/agent.go Outdated Show resolved Hide resolved
internal/license/agent.go Outdated Show resolved Hide resolved
internal/dataplane/parser/parser.go Outdated Show resolved Hide resolved
czeslavo and others added 2 commits June 15, 2023 12:56
Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com>
@czeslavo
Copy link
Contributor Author

The upstream endpoint apparently does weird things that it kinda shouldn't re 404s, but it looks like it was already kinda doing that. It's not clear whether the existing 404 case should have ever occurred in practice, but it was in the code, so 🤷

I was coding this following the GET /licenses behavior that the upstream PR is expected to introduce. I agree that 404 might be weird, but as I can see it's already there even without the changes in the linked PR (i.e. it returns 404 when it couldn't get the license singleton in Koko).

To address the concerns about the weirdness of 404 being handled in our code differently based on the error type down the stack, I refactored the license.Client to return mo.Option[KonnectLicense], err) which should better reflect the reality. It tells the caller that the license might be missing hence mo.Option (which is the case when we get 404). I also added tests there so that it's clear what's the expected behavior. PTAL 🙏

@czeslavo czeslavo requested a review from rainest June 15, 2023 13:17
@rainest rainest enabled auto-merge (squash) June 15, 2023 20:41
@rainest rainest merged commit 991908b into main Jun 15, 2023
35 checks passed
@rainest rainest deleted the license-polling-periods branch June 15, 2023 20:41
@github-actions
Copy link

The backport to release/2.10.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release/2.10.x release/2.10.x
# Navigate to the new working tree
cd .worktrees/backport-release/2.10.x
# Create a new branch
git switch --create backport-4178-to-release/2.10.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 991908b2e489add251064425e3879b5145a66dfa
# Push it to GitHub
git push --set-upstream origin backport-4178-to-release/2.10.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release/2.10.x

Then, create a pull request where the base branch is release/2.10.x and the compare/head branch is backport-4178-to-release/2.10.x.

pmalek pushed a commit that referenced this pull request Jun 20, 2023
Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com>
pmalek added a commit that referenced this pull request Jun 20, 2023
…d tests (#4172)

* tests: migrate GatewayClass related integration tests to envtest based tests

* chore(deps): bump k8s.io/code-generator in /third_party (#4189)

Bumps [k8s.io/code-generator](https://github.com/kubernetes/code-generator) from 0.27.2 to 0.27.3.
- [Commits](kubernetes/code-generator@v0.27.2...v0.27.3)

---
updated-dependencies:
- dependency-name: k8s.io/code-generator
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat(konnect): allow configuring license polling periods (#4178)


Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com>

* chore(deps): bump github.com/golangci/golangci-lint in /third_party (#4188)

Bumps [github.com/golangci/golangci-lint](https://github.com/golangci/golangci-lint) from 1.53.2 to 1.53.3.
- [Release notes](https://github.com/golangci/golangci-lint/releases)
- [Changelog](https://github.com/golangci/golangci-lint/blob/master/CHANGELOG.md)
- [Commits](golangci/golangci-lint@v1.53.2...v1.53.3)

---
updated-dependencies:
- dependency-name: github.com/golangci/golangci-lint
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump k8s.io/api from 0.27.2 to 0.27.3 (#4183)

Bumps [k8s.io/api](https://github.com/kubernetes/api) from 0.27.2 to 0.27.3.
- [Commits](kubernetes/api@v0.27.2...v0.27.3)

---
updated-dependencies:
- dependency-name: k8s.io/api
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump k8s.io/component-base from 0.27.2 to 0.27.3 (#4185)

Bumps [k8s.io/component-base](https://github.com/kubernetes/component-base) from 0.27.2 to 0.27.3.
- [Commits](kubernetes/component-base@v0.27.2...v0.27.3)

---
updated-dependencies:
- dependency-name: k8s.io/component-base
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump k8s.io/kubectl from 0.27.2 to 0.27.3 (#4187)

Bumps [k8s.io/kubectl](https://github.com/kubernetes/kubectl) from 0.27.2 to 0.27.3.
- [Commits](kubernetes/kubectl@v0.27.2...v0.27.3)

---
updated-dependencies:
- dependency-name: k8s.io/kubectl
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump google.golang.org/api from 0.127.0 to 0.128.0 (#4192)

Bumps [google.golang.org/api](https://github.com/googleapis/google-api-go-client) from 0.127.0 to 0.128.0.
- [Release notes](https://github.com/googleapis/google-api-go-client/releases)
- [Changelog](https://github.com/googleapis/google-api-go-client/blob/main/CHANGES.md)
- [Commits](googleapis/google-api-go-client@v0.127.0...v0.128.0)

---
updated-dependencies:
- dependency-name: google.golang.org/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump github.com/prometheus/client_golang (#4184)

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.15.1 to 1.16.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.15.1...v1.16.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* tests: fix flaky updates in TestGatewayFilters

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Burzyński <czeslavo@gmail.com>
Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com>
pmalek pushed a commit that referenced this pull request Jun 23, 2023
Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com>
pmalek added a commit that referenced this pull request Jun 26, 2023
Co-authored-by: Grzegorz Burzyński <czeslavo@gmail.com>
Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/feature New feature or request area/konnect Issues and PRs related to Konnect backport release/2.10.x size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Konnect license sync period(s) configurable
4 participants