Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Fix namespaced multiple binding/identity handling and verbose logs #388

Merged
merged 2 commits into from
Sep 19, 2019

Conversation

kkmsft
Copy link
Contributor

@kkmsft kkmsft commented Sep 19, 2019

In forcenamespaced configuration multiple bindings or identities could exists with the same name in different namespaces. The last binding which matched was used to create the assigned identity representing current state. This PR fixes the issue by using the right bindings and identities.

The map used for handling of identities did not consider the namespace. This PR changes the key of the map to a combination of namespace and name of the identity.

Reason for Change:

Issue Fixed:

Notes for Reviewers:

@kkmsft kkmsft requested a review from aramase September 19, 2019 01:01
Add more verbose logs for better debugging.

In forcenamespaced configuration multiple bindings or identities could exists with the same name in different namespaces. The last binding which matched was used to create the assigned identity representing current state. This PR fixes the issue by using the right bindings and identities.

The map used for handling of identities did not consider the namespace. This PR changes the key of the map to a combination of namespace and name of the identity.
@kkmsft
Copy link
Contributor Author

kkmsft commented Sep 19, 2019

Current test status:

Summarizing 2 Failures:

[Fail] Kubernetes cluster using aad-pod-identity [It] should not alter the user assigned identity on VM after AAD pod identity is created and deleted
/Users/kkmsft/projects/aad-pod-identity/go/src/github.com/Azure/aad-pod-identity/test/e2e/aadpodidentity_test.go:374

[Fail] Kubernetes cluster using aad-pod-identity [It] should create azureassignedidentities for 40 pods within ~2mins 30seconds
/Users/kkmsft/projects/aad-pod-identity/go/src/github.com/Azure/aad-pod-identity/test/e2e/aadpodidentity_test.go:1248

Ran 19 of 19 Specs in 5018.288 seconds
FAIL! -- 17 Passed | 2 Failed | 0 Pending | 0 Skipped
--- FAIL: TestAADPodIdentity (5018.29s)
FAIL
FAIL github.com/Azure/aad-pod-identity/test/e2e 5018.319s
make: *** [e2e] Error 1

@kkmsft
Copy link
Contributor Author

kkmsft commented Sep 19, 2019

• [SLOW TEST:297.169 seconds]
Kubernetes cluster using aad-pod-identity
/Users/kkmsft/projects/aad-pod-identity/go/src/github.com/Azure/aad-pod-identity/test/e2e/aadpodidentity_test.go:111
  should create azureassignedidentities for 40 pods within ~2mins 30seconds
  /Users/kkmsft/projects/aad-pod-identity/go/src/github.com/Azure/aad-pod-identity/test/e2e/aadpodidentity_test.go:555
------------------------------
SSSSSSSSS
Tearing down the test suite environment...
$ kubectl delete -f template/_output/default-deployment.yaml --now --ignore-not-found
$ kubectl delete -f template/busyboxds.yaml
$ kubectl delete deploy --all
$ kubectl get nodes -ojson
# Removing system assigned identity from k8s-agentpool1-31275592-vmss...
# Removing system assigned identity from k8s-agentpool1-31275592-vmss...
# Removing system assigned identity from k8s-agentpool1-31275592-vmss...

Ran 1 of 19 Specs in 436.890 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 18 Skipped
PASS
```bash

Copy link
Member

@aramase aramase left a comment

Choose a reason for hiding this comment

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

lgtm

@aramase aramase merged commit ffdad01 into Azure:master Sep 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants