Skip to content

Conversation

@m-messiah
Copy link
Member

What this PR does / why we need it:

The current cluster_proxy behaves differently from what is expected for empty kubeconfig variables in kubectl load rules.

The behaviour forbids the usage of cluster proxy inside the pod in the cluster.

This checks the possibility of in-cluster configuration and keeps the same behaviour, which is not found.

/area testing

@k8s-ci-robot k8s-ci-robot added area/testing Issues or PRs related to testing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 18, 2025
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 18, 2025
@m-messiah m-messiah force-pushed the allow-to-use-in-cluster-kubeconfig branch from 5ff9e33 to d8b6f51 Compare February 18, 2025 18:00
@sbueringer
Copy link
Member

/test pull-cluster-api-e2e-blocking-main

@sbueringer
Copy link
Member

/test pull-cluster-api-e2e-main

@sbueringer
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 20, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 439ad1d61b4c83b248895e91c1644cf43378221c

Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

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

/approve

@vincepri vincepri added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 20, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: vincepri

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

@k8s-ci-robot k8s-ci-robot merged commit f27917c into kubernetes-sigs:main Feb 20, 2025
19 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.10 milestone Feb 20, 2025
@mboersma
Copy link
Contributor

mboersma commented Apr 2, 2025

This was apparently a breaking change for CAPZ tests: see kubernetes-sigs/cluster-api-provider-azure#5530 and kubernetes-sigs/cluster-api-provider-azure#5533. I'd appreciate some advice on how to work around this change.

@m-messiah m-messiah deleted the allow-to-use-in-cluster-kubeconfig branch April 2, 2025 19:24
@sbueringer
Copy link
Member

sbueringer commented Apr 3, 2025

How did this break your test? I.e. in which case & how is it failing because of this change?

@mboersma
Copy link
Contributor

mboersma commented Apr 3, 2025

Apparently CAPZ was relying on the default behavior of NewClusterProxy when kubeconfig was not set, and that returns a different configuration because it's now calling rest.InClusterConfig(). All our e2e tests fail because they cannot provision a bootstrap cluster.

I think I've worked around it when updating to v1.9.6 by adding this line to our test setup before we create bootstrap clusters:

kubeconfigPath = clientcmd.NewDefaultClientConfigLoadingRules().GetDefaultFilename()

@sbueringer
Copy link
Member

sbueringer commented Apr 4, 2025

Ah, I see. That is a good point

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/testing Issues or PRs related to testing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants