-
Notifications
You must be signed in to change notification settings - Fork 366
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
[ExternalNode] Create Secret for vm-agent in RBAC #4560
Conversation
/test-vm-e2e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
@@ -194,7 +194,7 @@ function create_kubeconfig_files { | |||
# Kubeconfig to access K8S API | |||
|
|||
APISERVER=$(kubectl config view -o jsonpath="{.clusters[?(@.name==\"$CLUSTER_NAME\")].cluster.server}") | |||
TOKEN=$(kubectl -n $TEST_NAMESPACE get secrets -o jsonpath="{.items[?(@.metadata.annotations['kubernetes\.io/service-account\.name']=='$SERVICE_ACCOUNT')].data.token}"|base64 --decode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason to change it to use Secret Name instead of ServiceAccount Name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to be compatible with the original K8s versions. After we changed the RBAC file for VM Agent, a new Secret with name "vm-agent-service-account-token" is always created. It should work fine with newer versions in K8s cluster as there is only one Secret (what is created by us) is found with the annotation "kubernetes.io/service-account.name: vm-agent". For previous versions, a default Secret is created along with the SA, there would be two Secrets found with the annotation, then the final token string is incorrect as a concat of two tokens. As a result, I change the script to leverage the Secret which must exist in both previous version and later version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Thanks for the clarification.
Codecov Report
@@ Coverage Diff @@
## main #4560 +/- ##
==========================================
+ Coverage 64.58% 70.36% +5.78%
==========================================
Files 390 376 -14
Lines 57640 56059 -1581
==========================================
+ Hits 37225 39448 +2223
+ Misses 17760 13881 -3879
- Partials 2655 2730 +75
*This pull request uses carry forward flags. Click here to find out more.
|
/test-vm-e2e |
1. Crate a separate Secret for VM Agent in RBAC file, this because the Secret for a ServiceAccount is not created automatically since K8s v1.24. 2. Use the manually created Secret in Agent kubeconfig file. Signed-off-by: wenyingd <wenyingd@vmware.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/skip-all |
Fix #4558
Signed-off-by: wenyingd wenyingd@vmware.com