Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
81 changes: 44 additions & 37 deletions cmd/agent/app/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,46 +159,53 @@ func run(ctx context.Context, opts *options.Options) error {
if err != nil {
return fmt.Errorf("error building kubeconfig of member cluster: %w", err)
}
clusterKubeClient := kubeclientset.NewForConfigOrDie(clusterConfig)
controlPlaneKubeClient := kubeclientset.NewForConfigOrDie(controlPlaneRestConfig)
karmadaClient := karmadaclientset.NewForConfigOrDie(controlPlaneRestConfig)

registerOption := util.ClusterRegisterOption{
ClusterNamespace: opts.ClusterNamespace,
ClusterName: opts.ClusterName,
ReportSecrets: opts.ReportSecrets,
ClusterAPIEndpoint: opts.ClusterAPIEndpoint,
ProxyServerAddress: opts.ProxyServerAddress,
ClusterProvider: opts.ClusterProvider,
ClusterRegion: opts.ClusterRegion,
ClusterZones: opts.ClusterZones,
DryRun: false,
ControlPlaneConfig: controlPlaneRestConfig,
ClusterConfig: clusterConfig,
}
if opts.RegisterCluster {
clusterKubeClient := kubeclientset.NewForConfigOrDie(clusterConfig)
controlPlaneKubeClient := kubeclientset.NewForConfigOrDie(controlPlaneRestConfig)
karmadaClient := karmadaclientset.NewForConfigOrDie(controlPlaneRestConfig)

registerOption := util.ClusterRegisterOption{
ClusterNamespace: opts.ClusterNamespace,
ClusterName: opts.ClusterName,
ReportSecrets: opts.ReportSecrets,
ClusterAPIEndpoint: opts.ClusterAPIEndpoint,
ProxyServerAddress: opts.ProxyServerAddress,
ClusterProvider: opts.ClusterProvider,
ClusterRegion: opts.ClusterRegion,
ClusterZones: opts.ClusterZones,
DryRun: false,
ControlPlaneConfig: controlPlaneRestConfig,
ClusterConfig: clusterConfig,
}

registerOption.ClusterID, err = util.ObtainClusterID(clusterKubeClient)
if err != nil {
return err
}
registerOption.ClusterID, err = util.ObtainClusterID(clusterKubeClient)
if err != nil {
return err
}

if err = registerOption.Validate(karmadaClient, true); err != nil {
return err
}
if err = registerOption.Validate(karmadaClient, true); err != nil {
return err
}

clusterSecret, impersonatorSecret, err := util.ObtainCredentialsFromMemberCluster(clusterKubeClient, registerOption)
if err != nil {
return err
}
if clusterSecret != nil {
registerOption.Secret = *clusterSecret
}
if impersonatorSecret != nil {
registerOption.ImpersonatorSecret = *impersonatorSecret
}
err = util.RegisterClusterInControllerPlane(registerOption, controlPlaneKubeClient, generateClusterInControllerPlane)
if err != nil {
return fmt.Errorf("failed to register with karmada control plane: %w", err)
clusterSecret, impersonatorSecret, err := util.ObtainCredentialsFromMemberCluster(clusterKubeClient, registerOption)
if err != nil {
return err
}
if clusterSecret != nil {
registerOption.Secret = *clusterSecret
}
if impersonatorSecret != nil {
registerOption.ImpersonatorSecret = *impersonatorSecret
}
if err = util.RegisterClusterInControllerPlane(registerOption, controlPlaneKubeClient, generateClusterInControllerPlane); err != nil {
return fmt.Errorf("failed to register with karmada control plane: %w", err)
}
} else {
karmadaClient := karmadaclientset.NewForConfigOrDie(controlPlaneRestConfig)
memberKubeClient := kubeclientset.NewForConfigOrDie(clusterConfig)
if err = validateExternallyRegisteredCluster(ctx, opts, karmadaClient, memberKubeClient); err != nil {
return err
}
}

executionSpace := names.GenerateExecutionSpaceName(opts.ClusterName)
Expand Down
5 changes: 5 additions & 0 deletions cmd/agent/app/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ type Options struct {
CertRotationRemainingTimeThreshold float64
// KarmadaKubeconfigNamespace is the namespace of the secret containing karmada-agent certificate.
KarmadaKubeconfigNamespace string
// RegisterCluster indicates whether the agent should register the member cluster with the
// Karmada control plane on startup. When false, the cluster must already be registered in
// Pull mode and the agent only validates the existing registration before running its controllers.
RegisterCluster bool
}

// NewOptions builds an default scheduler options.
Expand Down Expand Up @@ -211,6 +215,7 @@ func (o *Options) AddFlags(fs *pflag.FlagSet, allControllers []string) {
fs.DurationVar(&o.CertRotationCheckingInterval, "cert-rotation-checking-interval", 5*time.Minute, "The interval of checking if the certificate need to be rotated. This is only applicable if cert rotation is enabled")
fs.Float64Var(&o.CertRotationRemainingTimeThreshold, "cert-rotation-remaining-time-threshold", 0.2, "The threshold of remaining time of the valid certificate. This is only applicable if cert rotation is enabled.")
fs.StringVar(&o.KarmadaKubeconfigNamespace, "karmada-kubeconfig-namespace", "karmada-system", "Namespace of the secret containing karmada-agent certificate. This is only applicable if cert rotation is enabled.")
fs.BoolVar(&o.RegisterCluster, "register-cluster", true, "Whether to register the member cluster with the Karmada control plane on startup. Set to false when the cluster is pre-registered by an external process; the agent will then only validate the existing Pull-mode registration and run its controllers.")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good that this is backwards compatible.

o.RateLimiterOpts.AddFlags(fs)
features.FeatureGate.AddFlag(fs)
o.ProfileOpts.AddFlags(fs)
Expand Down
64 changes: 64 additions & 0 deletions cmd/agent/app/registration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
Copyright 2021 The Karmada Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package app

import (
"context"
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubeclientset "k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"

"github.com/karmada-io/karmada/cmd/agent/app/options"
clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1"
karmadaclientset "github.com/karmada-io/karmada/pkg/generated/clientset/versioned"
"github.com/karmada-io/karmada/pkg/util"
)

// validateExternallyRegisteredCluster verifies that a cluster registered outside of the agent
// (i.e. with --register-cluster=false) is in a state the agent can run against: it exists, is not
// being deleted, uses Pull sync mode, and its ID matches the member cluster the agent is running for.
func validateExternallyRegisteredCluster(ctx context.Context, opts *options.Options, karmadaClient karmadaclientset.Interface, memberKubeClient kubeclientset.Interface) error {
cluster, err := karmadaClient.ClusterV1alpha1().Clusters().Get(ctx, opts.ClusterName, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to get cluster %q from control plane: %w", opts.ClusterName, err)
}

if !cluster.DeletionTimestamp.IsZero() {
return fmt.Errorf("cluster %q is being deleted", opts.ClusterName)
}

if cluster.Spec.SyncMode != clusterv1alpha1.Pull {
return fmt.Errorf("cluster %q has SyncMode %q, expected %q for an externally registered cluster", opts.ClusterName, cluster.Spec.SyncMode, clusterv1alpha1.Pull)
}

clusterID, err := util.ObtainClusterID(memberKubeClient)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can drop the cluster ID check. The ID is an optional field that may not always be set. Also this util function does not take into account the fact that there's multiple ways in which it can be set.

Ref: https://github.com/karmada-io/karmada/blob/release-1.18/pkg/apis/cluster/types.go#L64

Or maybe we can make the util function more robust.

@RainbowMango , what do you think?

if err != nil {
return fmt.Errorf("failed to obtain cluster ID from member cluster: %w", err)
}

if cluster.Spec.ID != "" && cluster.Spec.ID != clusterID {
return fmt.Errorf("cluster ID mismatch: control plane has %q but member cluster reports %q", cluster.Spec.ID, clusterID)
}
if cluster.Spec.ID == "" {
klog.Warningf("Cluster %q has no ID set in the control plane; consider setting spec.id to %q", opts.ClusterName, clusterID)
}

klog.Infof("Successfully validated externally registered cluster %q", opts.ClusterName)
return nil
}
164 changes: 164 additions & 0 deletions cmd/agent/app/registration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
/*
Copyright 2021 The Karmada Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package app

import (
"context"
"strings"
"testing"
"time"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
kubefake "k8s.io/client-go/kubernetes/fake"

"github.com/karmada-io/karmada/cmd/agent/app/options"
clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1"
karmadafake "github.com/karmada-io/karmada/pkg/generated/clientset/versioned/fake"
)

func newTestOpts(clusterName string) *options.Options {
return &options.Options{
ClusterName: clusterName,
}
}

func newKubeSystemNamespace(uid string) *corev1.Namespace {
return &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: metav1.NamespaceSystem,
UID: types.UID(uid),
},
}
}

func newCluster(name string, syncMode clusterv1alpha1.ClusterSyncMode, id string) *clusterv1alpha1.Cluster {
return &clusterv1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Spec: clusterv1alpha1.ClusterSpec{
SyncMode: syncMode,
ID: id,
},
}
}

func TestValidateExternallyRegisteredCluster(t *testing.T) {
const (
clusterName = "member1"
clusterUID = "test-uid-12345"
)

tests := []struct {
name string
cluster *clusterv1alpha1.Cluster
nsUID string
noKubeSystemNS bool
wantErr bool
errContains string
}{
{
name: "happy path with matching ID",
cluster: newCluster(clusterName, clusterv1alpha1.Pull, clusterUID),
nsUID: clusterUID,
wantErr: false,
},
{
name: "happy path with empty cluster ID",
cluster: newCluster(clusterName, clusterv1alpha1.Pull, ""),
nsUID: clusterUID,
wantErr: false,
},
{
name: "cluster not found",
cluster: nil,
nsUID: clusterUID,
wantErr: true,
errContains: "failed to get cluster",
},
{
name: "cluster being deleted",
cluster: func() *clusterv1alpha1.Cluster {
c := newCluster(clusterName, clusterv1alpha1.Pull, clusterUID)
now := metav1.NewTime(time.Now())
c.DeletionTimestamp = &now
c.Finalizers = []string{"test-finalizer"}
return c
}(),
nsUID: clusterUID,
wantErr: true,
errContains: "is being deleted",
},
{
name: "wrong sync mode (Push)",
cluster: newCluster(clusterName, clusterv1alpha1.Push, clusterUID),
nsUID: clusterUID,
wantErr: true,
errContains: "SyncMode",
},
{
name: "cluster ID mismatch",
cluster: newCluster(clusterName, clusterv1alpha1.Pull, "different-uid"),
nsUID: clusterUID,
wantErr: true,
errContains: "cluster ID mismatch",
},
{
name: "member cluster missing kube-system namespace",
cluster: newCluster(clusterName, clusterv1alpha1.Pull, clusterUID),
noKubeSystemNS: true,
wantErr: true,
errContains: "failed to obtain cluster ID",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var karmadaClient *karmadafake.Clientset
if tt.cluster != nil {
karmadaClient = karmadafake.NewSimpleClientset(tt.cluster)
} else {
karmadaClient = karmadafake.NewSimpleClientset()
}

var memberKubeClient *kubefake.Clientset
if tt.noKubeSystemNS {
memberKubeClient = kubefake.NewSimpleClientset()
} else {
memberKubeClient = kubefake.NewSimpleClientset(newKubeSystemNamespace(tt.nsUID))
}
opts := newTestOpts(clusterName)

err := validateExternallyRegisteredCluster(context.Background(), opts, karmadaClient, memberKubeClient)

if tt.wantErr {
if err == nil {
t.Fatal("expected error, got nil")
}
if tt.errContains != "" && !strings.Contains(err.Error(), tt.errContains) {
t.Errorf("expected error containing %q, got %q", tt.errContains, err.Error())
}
} else {
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
})
}
}
1 change: 1 addition & 0 deletions docs/command-line-flags/karmada-agent.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ Generic flags:
--rate-limiter-bucket-size int The bucket size for rate limier. (default 100)
--rate-limiter-max-delay duration The max delay for rate limiter. (default 16m40s)
--rate-limiter-qps int The QPS for rate limier. (default 10)
--register-cluster Whether to register the member cluster with the Karmada control plane on startup. Set to false when the cluster is pre-registered by an external process; the agent will then only validate the existing Pull-mode registration and run its controllers. (default true)
--report-secrets strings The secrets that are allowed to be reported to the Karmada control plane during registering. Valid values are 'KubeCredentials', 'KubeImpersonator' and 'None'. e.g 'KubeCredentials,KubeImpersonator' or 'None'. (default [KubeCredentials,KubeImpersonator])
--resync-period duration Base frequency the informers are resynced.
```
Expand Down