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

Do not use default namespace in AKS e2e example #1149

Closed
wants to merge 1 commit into from

Conversation

mjura
Copy link
Contributor

@mjura mjura commented Mar 17, 2025

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

Sorry, something went wrong.

@mjura mjura requested a review from a team as a code owner March 17, 2025 15:08
@mjura mjura self-assigned this Mar 17, 2025
@anmazzotti
Copy link
Contributor

@mjura Please check if #1142 already includes these changes. If so I think this one can be closed.

Danil-Grigorev
Danil-Grigorev previously approved these changes Mar 18, 2025
@mjura mjura force-pushed the e2e-aks-namespace branch from 8c47bf2 to 42dc941 Compare March 18, 2025 14:50
spec:
allowedNamespaces: {}
clientID: ${AZURE_CLIENT_ID}
clientSecret:
name: cluster-identity-secret
namespace: capz-system
namespace: ${NAMESPACE}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to break the test. The Secret is actually in the capz-system namespace (it's reused across tests).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According our documentation: Note that the AzureClusterIdentity is a namespaced resource and it needs to be created in the same namespace as the Cluster. https://turtles.docs.rancher.com/turtles/next/en/user/clusterclass.html#_setup

Copy link
Contributor

@anmazzotti anmazzotti Mar 20, 2025

Choose a reason for hiding this comment

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

@mjura The documentation is correct. The line I mentioned however is a Secret reference.
The Secret is in capz-system, the AzureClusterIdentity is not.

@alexander-demicev alexander-demicev added the area/testing Indicates an issue related to test label Mar 19, 2025
@salasberryfin
Copy link
Contributor

@mjura looks like this may need a change after merging #1160.

@mjura mjura closed this Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Indicates an issue related to test
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants