Skip to content

✨ Add e2e tests for frontproxy objects #38

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
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

✨ Add e2e tests for frontproxy objects #38

wants to merge 10 commits into from
+149 −4

Conversation

SimonTheLeg
Copy link
Contributor

@SimonTheLeg SimonTheLeg commented Feb 10, 2025 •
edited
Loading

Summary

Adds simple Rootshard + Frontproxy E2E testcase

Related issue(s)

Fixes #28

Notes

Should be merged after #40 is merged and rebased onto this PR

Release Notes

NONE

@kcp-ci-bot kcp-ci-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Feb 10, 2025
@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign clubanderson for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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 dco-signoff: no Indicates the PR's author has not signed the DCO. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 10, 2025
@kcp-ci-bot
Copy link
Contributor

Hi @SimonTheLeg. Thanks for your PR.

I'm waiting for a kcp-dev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kcp-ci-bot kcp-ci-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 10, 2025
@SimonTheLeg SimonTheLeg changed the title Add e2e test for simple fp creation ✨ Add e2e tests for frontproxy creation Feb 10, 2025
@SimonTheLeg SimonTheLeg force-pushed the front-proxy-e2e-tests branch from fe98b7b to 2a00b74 Compare February 10, 2025 12:11
@kcp-ci-bot kcp-ci-bot added dco-signoff: yes Indicates the PR's author has signed the DCO. and removed dco-signoff: no Indicates the PR's author has not signed the DCO. labels Feb 10, 2025
@SimonTheLeg SimonTheLeg changed the title ✨ Add e2e tests for frontproxy creation ✨ Add e2e tests for frontproxy objects Feb 10, 2025
@SimonTheLeg SimonTheLeg mentioned this pull request Mar 14, 2025
Merged
@SimonTheLeg SimonTheLeg force-pushed the front-proxy-e2e-tests branch from 2a00b74 to 9f8d796 Compare March 14, 2025 17:44
@SimonTheLeg SimonTheLeg marked this pull request as ready for review March 14, 2025 17:47
@kcp-ci-bot kcp-ci-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 14, 2025
@mjudeikis
Copy link

/ok-to-test

@kcp-ci-bot kcp-ci-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 14, 2025
@SimonTheLeg
Add e2e test for simple fp creation
15f9b00 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
make self-desctructing namespaces optional
ce33dfa 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
Revert "make self-desctructing namespaces optional"
d3d1806 
This reverts commit ffe3460.

On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
point BinaryAssetDirectory for Ginkgo to new tools directory
742b359 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
add instructions to running E2E tests locally
2073975 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
template frontproxy externalhostname on frontproxy targeting kubeconfigs
3b98e30 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
make frontproxy tests work
04cf36c 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg SimonTheLeg force-pushed the front-proxy-e2e-tests branch from d5ff76b to 04cf36c Compare March 21, 2025 12:51
@SimonTheLeg
restrict the overly aggressive markdown linting
73b458e 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
importorder
97da797 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
Copy link
Contributor Author

/retest

so the logs become available again

@SimonTheLeg SimonTheLeg force-pushed the front-proxy-e2e-tests branch from cdd36db to 0567b0d Compare April 4, 2025 13:37
@SimonTheLeg
increase timeout
6b44d71 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg SimonTheLeg force-pushed the front-proxy-e2e-tests branch from 0567b0d to 6b44d71 Compare April 4, 2025 13:55
@SimonTheLeg
Copy link
Contributor Author

Okay it was just the timeout on WaitFor which was a bit too short sometimes leading to failing tests.

Now everything works @embik @xrstf: Ready for review, whoever wants it :)

@SimonTheLeg SimonTheLeg requested review from embik and xrstf April 7, 2025 07:50
embik
embik reviewed Apr 7, 2025
View reviewed changes
internal/controller/kubeconfig_controller.go
Comment on lines -130 to +131
serverURL = fmt.Sprintf("https://%s:6443", rootShard.Spec.External.Hostname)
serverName = rootShard.Spec.External.Hostname
serverURL = fmt.Sprintf("https://%s:6443", frontProxy.Spec.ExternalHostname)
serverName = frontProxy.Spec.ExternalHostname
Copy link
Member

Choose a reason for hiding this comment

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

What's up with this change? The hostname should be preferring the shard external hostname, because any interaction via kubectl plugins with use the external hostname exposed by the shard anyway. If I remember correctly we talked about this a while ago, in the current kcp way of doing things, this won't really work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@embik embik embik left review comments

@xrstf xrstf Awaiting requested review from xrstf

Assignees
No one assigned
Labels
dco-signoff: yes Indicates the PR's author has signed the DCO. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

chore: add e2e test for FrontProxy objects
4 participants
@SimonTheLeg @kcp-ci-bot @mjudeikis @embik