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

🐛 Failed to sync sa work-controller-sa in cluster manger hosted mode #223

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions deploy/cluster-manager/config/rbac/cluster_role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ rules:
- "work-webhook-sa-kubeconfig"
- "placement-controller-sa-kubeconfig"
- "work-controller-sa-kubeconfig"
- "addon-manager-controller-sa-kubeconfig"
- "external-hub-kubeconfig"
- apiGroups: [""]
resources: ["secrets"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ spec:
- work-webhook-sa-kubeconfig
- placement-controller-sa-kubeconfig
- work-controller-sa-kubeconfig
- addon-manager-controller-sa-kubeconfig
- external-hub-kubeconfig
resources:
- secrets
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ type clusterManagerController struct {
cache resourceapply.ResourceCache
// For testcases which don't need these functions, we could set fake funcs
ensureSAKubeconfigs func(ctx context.Context, clusterManagerName, clusterManagerNamespace string,
hubConfig *rest.Config, hubClient, managementClient kubernetes.Interface, recorder events.Recorder) error
hubConfig *rest.Config, hubClient, managementClient kubernetes.Interface, recorder events.Recorder,
mwctrEnabled, addonManagerEnabled bool) error
Copy link
Member

Choose a reason for hiding this comment

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

can we just use features? it is global variable, we do not need them as input

Copy link
Member Author

@zhujian7 zhujian7 Jul 14, 2023

Choose a reason for hiding this comment

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

for hosted mode, there might be multiple clusterManager CRs, if they have different feature gate settings, I don't think using the global variable is appropriate.

generateHubClusterClients func(hubConfig *rest.Config) (kubernetes.Interface, apiextensionsclient.Interface,
migrationclient.StorageVersionMigrationsGetter, error)
skipRemoveCRDs bool
Expand Down Expand Up @@ -298,8 +299,9 @@ func generateHubClients(hubKubeConfig *rest.Config) (kubernetes.Interface, apiex
// We create a ServiceAccount with a rolebinding on the hub cluster, and then use the token of the ServiceAccount as the user of the kubeconfig.
// Finally, a deployment on the management cluster would use the kubeconfig to access resources on the hub cluster.
func ensureSAKubeconfigs(ctx context.Context, clusterManagerName, clusterManagerNamespace string,
hubKubeConfig *rest.Config, hubClient, managementClient kubernetes.Interface, recorder events.Recorder) error {
for _, sa := range getSAs() {
hubKubeConfig *rest.Config, hubClient, managementClient kubernetes.Interface, recorder events.Recorder,
mwctrEnabled, addonManagerEnabled bool) error {
for _, sa := range getSAs(mwctrEnabled, addonManagerEnabled) {
tokenGetter := helpers.SATokenCreater(ctx, sa, clusterManagerNamespace, hubClient)
err := helpers.SyncKubeConfigSecret(ctx, sa+"-kubeconfig", clusterManagerNamespace, "/var/run/secrets/hub/kubeconfig", &rest.Config{
Host: hubKubeConfig.Host,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,10 @@ func setup(t *testing.T, tc *testController, cd []runtime.Object, crds ...runtim
tc.clusterManagerController.generateHubClusterClients = func(hubKubeConfig *rest.Config) (kubernetes.Interface, apiextensionsclient.Interface, migrationclient.StorageVersionMigrationsGetter, error) {
return fakeHubKubeClient, fakeAPIExtensionClient, fakeMigrationClient.MigrationV1alpha1(), nil
}
tc.clusterManagerController.ensureSAKubeconfigs = func(ctx context.Context, clusterManagerName, clusterManagerNamespace string, hubConfig *rest.Config, hubClient, managementClient kubernetes.Interface, recorder events.Recorder) error {
tc.clusterManagerController.ensureSAKubeconfigs = func(ctx context.Context,
clusterManagerName, clusterManagerNamespace string, hubConfig *rest.Config,
hubClient, managementClient kubernetes.Interface, recorder events.Recorder,
mwctrEnabled, addonManagerEnabled bool) error {
return nil
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ type runtimeReconcile struct {
hubKubeConfig *rest.Config

ensureSAKubeconfigs func(ctx context.Context, clusterManagerName, clusterManagerNamespace string,
hubConfig *rest.Config, hubClient, managementClient kubernetes.Interface, recorder events.Recorder) error
hubConfig *rest.Config, hubClient, managementClient kubernetes.Interface, recorder events.Recorder,
mwctrEnabled, addonManagerEnabled bool) error

cache resourceapply.ResourceCache
recorder events.Recorder
Expand Down Expand Up @@ -77,7 +78,9 @@ func (c *runtimeReconcile) reconcile(ctx context.Context, cm *operatorapiv1.Clus
// Before this step, the serviceaccounts in the hub cluster and the namespace in the management cluster should be applied first.
if cm.Spec.DeployOption.Mode == operatorapiv1.InstallModeHosted {
clusterManagerNamespace := helpers.ClusterManagerNamespace(cm.Name, cm.Spec.DeployOption.Mode)
err := c.ensureSAKubeconfigs(ctx, cm.Name, clusterManagerNamespace, c.hubKubeConfig, c.hubKubeClient, c.kubeClient, c.recorder)
err := c.ensureSAKubeconfigs(ctx, cm.Name, clusterManagerNamespace,
c.hubKubeConfig, c.hubKubeClient, c.kubeClient, c.recorder,
config.MWReplicaSetEnabled, config.AddOnManagerEnabled)
if err != nil {
meta.SetStatusCondition(&cm.Status.Conditions, metav1.Condition{
Type: clusterManagerApplied,
Expand Down Expand Up @@ -190,12 +193,18 @@ func (c *runtimeReconcile) clean(ctx context.Context, cm *operatorapiv1.ClusterM
}

// getSAs return serviceaccount names of all hub components
func getSAs() []string {
return []string{
func getSAs(mwctrEnabled, addonManagerEnabled bool) []string {
sas := []string{
"registration-controller-sa",
"registration-webhook-sa",
"work-webhook-sa",
"placement-controller-sa",
"work-controller-sa",
}
if mwctrEnabled {
sas = append(sas, "work-controller-sa")
}
if addonManagerEnabled {
sas = append(sas, "addon-manager-controller-sa")
}
return sas
}