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

Implement karmadactl get to support pull mode cluster #1575

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

lonelyCZ
Copy link
Member

@lonelyCZ lonelyCZ commented Apr 1, 2022

Signed-off-by: lonelyCZ [email protected]

What type of PR is this?
/kind feature

What this PR does / why we need it:
Implement karmadactl get to support pull mode cluster through aggregated-apiserver.

Which issue(s) this PR fixes:
Fixes #1091

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Implement `karmadactl get` to support pull mode cluster

@karmada-bot karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 1, 2022
@karmada-bot karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 1, 2022
@RainbowMango
Copy link
Member

Wow, nice!!!
/assign @XiShanYongYe-Chang

Comment on lines 379 to 395
// create a ClusterRole in cluster.
clusterRole := &rbacv1.ClusterRole{}
clusterRole.Name = names.GenerateRoleName(impersonationSA.Name)
clusterRole.Rules = clusterPolicyRules
if _, err = ensureClusterRoleExist(clusterKubeClient, clusterRole); err != nil {
return nil, err
}

// create a ClusterRoleBinding in cluster.
clusterRoleBinding := &rbacv1.ClusterRoleBinding{}
clusterRoleBinding.Name = clusterRole.Name
clusterRoleBinding.Subjects = buildRoleBindingSubjects(impersonationSA.Name, impersonationSA.Namespace)
clusterRoleBinding.RoleRef = buildClusterRoleReference(clusterRole.Name)
if _, err = ensureClusterRoleBindingExist(clusterKubeClient, clusterRoleBinding); err != nil {
return nil, err
}

Copy link
Member

Choose a reason for hiding this comment

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

We have create ClusterRole karmada-impersonator and ClusterRoleBinding karmada-impersonator for pull mode cluster, perhaps we do not need to create those rbac resource.

# kubectl --kubeconfig /root/.kube/members.config --context member3 get clusterrole | grep karmada-impersonator
karmada-impersonator                                                   2022-04-01T08:02:54Z
# kubectl --kubeconfig /root/.kube/members.config --context member3 get clusterrolebinding | grep karmada-impersonator
karmada-impersonator  

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I didn't see where to create these rbac, and let me see again.

Copy link
Member Author

Choose a reason for hiding this comment

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

@XiShanYongYe-Chang I still can't find the exact location that create rbac for Pull cluster.

Copy link
Member

Choose a reason for hiding this comment

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

Look here: pkg/controllers/unifiedauth/unified_auth_controller.go

unified-auth-controller will create impersonator rbac for all cluster(include pull mode).

Copy link
Member Author

Choose a reason for hiding this comment

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

Does this controller(unified-auth-controller) need to be registered in agent component? Or directly manipulate PULL mode clusters in karmada-controller-manager component?

Copy link
Member

Choose a reason for hiding this comment

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

unified-auth-controller will create work with clusterrole/clusterrolebinding for all cluster:

// step5: sync clusterrole to cluster for impersonation
if err := c.buildImpersonationClusterRole(cluster, rules); err != nil {
klog.Errorf("failed to sync impersonate clusterrole to cluster(%s): %v", cluster.Name, err)
return err
}
// step6: sync clusterrolebinding to cluster for impersonation
if err := c.buildImpersonationClusterRoleBinding(cluster); err != nil {
klog.Errorf("failed to sync impersonate clusterrolebinding to cluster(%s): %v", cluster.Name, err)
return err
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Have removed it.

@karmada-bot karmada-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 2, 2022
Copy link
Member

@XiShanYongYe-Chang XiShanYongYe-Chang left a comment

Choose a reason for hiding this comment

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

good job!

/lgtm

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 2, 2022
@XiShanYongYe-Chang
Copy link
Member

Test ok:

# ./karmadactl get po -n kube-system
NAME                                            CLUSTER   READY   STATUS    RESTARTS   AGE
coredns-64897985d-qvvpg                         member3   1/1     Running   0          12m
coredns-64897985d-xrt7g                         member3   1/1     Running   0          12m
etcd-member3-control-plane                      member3   1/1     Running   0          12m
kindnet-4qkq5                                   member3   1/1     Running   0          12m
kube-apiserver-member3-control-plane            member3   1/1     Running   0          12m
kube-controller-manager-member3-control-plane   member3   1/1     Running   0          12m
kube-proxy-rnrvz                                member3   1/1     Running   0          12m
kube-scheduler-member3-control-plane            member3   1/1     Running   0          12m
coredns-64897985d-5znl9                         member2   1/1     Running   0          12m
coredns-64897985d-rwj96                         member2   1/1     Running   0          12m
etcd-member2-control-plane                      member2   1/1     Running   0          12m
kindnet-24xj9                                   member2   1/1     Running   0          12m
kube-apiserver-member2-control-plane            member2   1/1     Running   0          12m
kube-controller-manager-member2-control-plane   member2   1/1     Running   0          12m
kube-proxy-ztwhq                                member2   1/1     Running   0          12m
kube-scheduler-member2-control-plane            member2   1/1     Running   0          12m
coredns-64897985d-gsl52                         member1   1/1     Running   0          12m
coredns-64897985d-rtjl2                         member1   1/1     Running   0          12m
etcd-member1-control-plane                      member1   1/1     Running   0          12m
kindnet-qccrb                                   member1   1/1     Running   0          12m
kube-apiserver-member1-control-plane            member1   1/1     Running   0          12m
kube-controller-manager-member1-control-plane   member1   1/1     Running   0          12m
kube-proxy-7fzfx                                member1   1/1     Running   0          12m
kube-scheduler-member1-control-plane            member1   1/1     Running   0          12m

Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/approve

@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 6, 2022
@karmada-bot karmada-bot merged commit 79442df into karmada-io:master Apr 6, 2022
@lonelyCZ lonelyCZ deleted the pr-get-pull branch July 11, 2022 08:01
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. kind/feature Categorizes issue or PR as related to a new feature. lgtm 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.

Enable karmadactl get to support pull mode.
4 participants