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

🐛 Generate admin.kubeconfig with correct cluster for system:admin context #3070

Merged
merged 1 commit into from
Jan 4, 2024
Merged

🐛 Generate admin.kubeconfig with correct cluster for system:admin context #3070

merged 1 commit into from
Jan 4, 2024

Conversation

embik
Copy link
Member

@embik embik commented Jan 3, 2024 •
edited
Loading

Summary

A long while ago, / redirected to the system:admin workspace. That is no longer the case, and as such the admin.kubeconfig generated by kcp should have a separate cluster/server from the base one, so that the system:admin context in it is functional.

I'm also adding a shard-base context because the combination of "base" server and "shard-admin" credentials are required in a lot of testing and sharding.

I also found this outdated information in the docs, so replaced it with some more up-to-date information.

Related issue(s)

Fixes #

Release Notes

Fix `system:admin` context and add `system:base` in generated `admin.kubeconfig`

@embik embik requested review from sttts and mjudeikis January 3, 2024 09:29
@embik embik self-assigned this Jan 3, 2024
@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has signed the DCO. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 3, 2024
@embik embik changed the title 🐛 generate admin.kubeconfig with correct cluster for server:admin context 🐛 Generate admin.kubeconfig with correct cluster for server:admin context Jan 3, 2024
@embik
Copy link
Member Author

embik commented Jan 3, 2024

I guess this is breaking e2e tests, I will need to check why.

@sttts
Copy link
Member

sttts commented Jan 3, 2024

Lgtm. Any idea why e2e break?

@embik
Copy link
Member Author

embik commented Jan 3, 2024

Lgtm. Any idea why e2e break?

Looking into it. I can replicate them on my local system, so I assume it's not a flake.

@embik
Copy link
Member Author

embik commented Jan 3, 2024 •
edited
Loading

Should work now. The e2e framework needed the shard-admin credentials and was "utilising" the fact that the system:admin context was using / as server, so now I need to fetch the base URL from the base context and combine the two.

edit: Looks like the fix only worked on some tests.

@embik embik force-pushed the kubeconfig-system-admin-cluster branch 2 times, most recently from 7d5bc5a to ed0362d Compare January 4, 2024 08:17
@kcp-ci-bot kcp-ci-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 4, 2024
@embik embik changed the title 🐛 Generate admin.kubeconfig with correct cluster for server:admin context 🐛 Generate admin.kubeconfig with correct cluster for system:admin context Jan 4, 2024
@embik
Copy link
Member Author

embik commented Jan 4, 2024

@sttts I had to rework the PR a bit. Turns out sharding is looking for the system:admin context in kubeconfigs and started failing because of that.

I've resolved this problem by creating a new system:base context that is not usable from kubectl (just as the base one), but that is now used by sharding and testing code. PTAL.

sttts
sttts reviewed Jan 4, 2024
View reviewed changes
@embik embik force-pushed the kubeconfig-system-admin-cluster branch 2 times, most recently from 091a0d1 to 9189f3f Compare January 4, 2024 12:30
@embik embik force-pushed the kubeconfig-system-admin-cluster branch from 9189f3f to 5becfac Compare January 4, 2024 12:36
@sttts
Copy link
Member

sttts commented Jan 4, 2024

/lgtm
/approve

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 4, 2024
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 3b6889af8016ba67f9ab11a252d672361746c325

@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 4, 2024
@kcp-ci-bot kcp-ci-bot merged commit 1c23d11 into kcp-dev:main Jan 4, 2024
6 checks passed
@embik embik deleted the kubeconfig-system-admin-cluster branch January 4, 2024 13:42
@embik
Copy link
Member Author

embik commented Jan 24, 2024

/kind bug

@kcp-ci-bot kcp-ci-bot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts sttts left review comments

@mjudeikis mjudeikis Awaiting requested review from mjudeikis

Assignees

@sttts sttts

@embik embik

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@embik @sttts @kcp-ci-bot