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

External etcd Support for Karmada Operator #5536

Closed
wants to merge 7 commits into from
Closed
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
24 changes: 21 additions & 3 deletions charts/karmada-operator/crds/operator.karmada.io_karmadas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@ spec:
description: |-
CAData is an SSL Certificate Authority file used to secure etcd communication.
Required if using a TLS connection.
Deprecated: This field is deprecated and will be removed in a future version. Use SecretRef for providing client connection credentials.
format: byte
type: string
certData:
description: |-
CertData is an SSL certification file used to secure etcd communication.
Required if using a TLS connection.
Deprecated: This field is deprecated and will be removed in a future version. Use SecretRef for providing client connection credentials.
format: byte
type: string
endpoints:
Expand All @@ -82,13 +84,29 @@ spec:
description: |-
KeyData is an SSL key file used to secure etcd communication.
Required if using a TLS connection.
Deprecated: This field is deprecated and will be removed in a future version. Use SecretRef for providing client connection credentials.
format: byte
type: string
secretRef:
description: |-
SecretRef references a Kubernetes secret containing the etcd connection credentials.
The secret must contain the following data keys:
ca.crt: The Certificate Authority (CA) certificate data.
tls.crt: The TLS certificate data used for verifying the etcd server's certificate.
tls.key: The TLS private key.
Required to configure the connection to an external etcd cluster.
properties:
name:
description: Name is the name of resource being referenced.
type: string
namespace:
description: Namespace is the namespace for the resource
being referenced.
type: string
type: object
required:
- caData
- certData
- endpoints
- keyData
- secretRef
type: object
local:
description: |-
Expand Down
24 changes: 21 additions & 3 deletions operator/config/crds/operator.karmada.io_karmadas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@ spec:
description: |-
CAData is an SSL Certificate Authority file used to secure etcd communication.
Required if using a TLS connection.
Deprecated: This field is deprecated and will be removed in a future version. Use SecretRef for providing client connection credentials.
format: byte
type: string
certData:
description: |-
CertData is an SSL certification file used to secure etcd communication.
Required if using a TLS connection.
Deprecated: This field is deprecated and will be removed in a future version. Use SecretRef for providing client connection credentials.
format: byte
type: string
endpoints:
Expand All @@ -82,13 +84,29 @@ spec:
description: |-
KeyData is an SSL key file used to secure etcd communication.
Required if using a TLS connection.
Deprecated: This field is deprecated and will be removed in a future version. Use SecretRef for providing client connection credentials.
format: byte
type: string
secretRef:
description: |-
SecretRef references a Kubernetes secret containing the etcd connection credentials.
The secret must contain the following data keys:
ca.crt: The Certificate Authority (CA) certificate data.
tls.crt: The TLS certificate data used for verifying the etcd server's certificate.
tls.key: The TLS private key.
Required to configure the connection to an external etcd cluster.
properties:
name:
description: Name is the name of resource being referenced.
type: string
namespace:
description: Namespace is the namespace for the resource
being referenced.
type: string
type: object
required:
- caData
- certData
- endpoints
- keyData
- secretRef
type: object
local:
description: |-
Expand Down
1 change: 1 addition & 0 deletions operator/pkg/apis/operator/v1alpha1/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ func setDefaultsEtcd(obj *KarmadaComponents) {
obj.Etcd = &Etcd{}
}

// If external etcd client connection is not configured, we'll default to using in-cluster etcd configuration
if obj.Etcd.External == nil {
if obj.Etcd.Local == nil {
obj.Etcd.Local = &LocalEtcd{}
Expand Down
21 changes: 17 additions & 4 deletions operator/pkg/apis/operator/v1alpha1/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,22 +236,35 @@ type VolumeData struct {
}

// ExternalEtcd describes an external etcd cluster.
// operator has no knowledge of where certificate files live, and they must be supplied.
// Operator has no knowledge of where certificate files live, and they must be supplied.
type ExternalEtcd struct {
// Endpoints of etcd members. Required for ExternalEtcd.
// +required
Endpoints []string `json:"endpoints"`

// CAData is an SSL Certificate Authority file used to secure etcd communication.
// Required if using a TLS connection.
CAData []byte `json:"caData"`
// Deprecated: This field is deprecated and will be removed in a future version. Use SecretRef for providing client connection credentials.
CAData []byte `json:"caData,omitempty"`

// CertData is an SSL certification file used to secure etcd communication.
// Required if using a TLS connection.
CertData []byte `json:"certData"`
// Deprecated: This field is deprecated and will be removed in a future version. Use SecretRef for providing client connection credentials.
CertData []byte `json:"certData,omitempty"`

// KeyData is an SSL key file used to secure etcd communication.
// Required if using a TLS connection.
KeyData []byte `json:"keyData"`
// Deprecated: This field is deprecated and will be removed in a future version. Use SecretRef for providing client connection credentials.
KeyData []byte `json:"keyData,omitempty"`

// SecretRef references a Kubernetes secret containing the etcd connection credentials.
// The secret must contain the following data keys:
// ca.crt: The Certificate Authority (CA) certificate data.
// tls.crt: The TLS certificate data used for verifying the etcd server's certificate.
// tls.key: The TLS private key.
// Required to configure the connection to an external etcd cluster.
RainbowMango marked this conversation as resolved.
Show resolved Hide resolved
// +required
SecretRef LocalSecretReference `json:"secretRef"`
}

// KarmadaAPIServer holds settings to kube-apiserver component of the kubernetes.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions operator/pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ const (
KarmadaAPIserverListenClientPort = 5443
// EtcdDataVolumeName defines the name to etcd data volume
EtcdDataVolumeName = "etcd-data"
// EtcClientCredentialsVolumeName defines the name of the volume for the etcd client credentials
EtcClientCredentialsVolumeName = "etcd-client-credentials" // #nosec G101
// EtcClientCredentialsMountPath defines the mount path for the etcd client credentials data
EtcClientCredentialsMountPath = "/etc/etcd/pki" // #nosec G101
Comment on lines +80 to +83
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// EtcClientCredentialsVolumeName defines the name of the volume for the etcd client credentials
EtcClientCredentialsVolumeName = "etcd-client-credentials" // #nosec G101
// EtcClientCredentialsMountPath defines the mount path for the etcd client credentials data
EtcClientCredentialsMountPath = "/etc/etcd/pki" // #nosec G101
// EtcClientCredentialsVolumeName defines the name of the volume for the etcd client credentials
EtcClientCredentialsVolumeName = "etcd-client-cert" // #nosec G101
// EtcClientCredentialsMountPath defines the mount path for the etcd client credentials data
EtcClientCredentialsMountPath = "/etc/karmada/pki/etcd-client" // #nosec G101

As discussed with @chaosi-zju, we are going to standardize the volume name and mount path like this:

  • Volume: <server>-client-cert // e.g. etcd-client-cert
  • Mount path: /etc/karmada/pki/<server> // e.g. /etc/karmada/pki/etcd-client
  • File in container:
    • /etc/karmada/pki/<server>-client/ca.crt // e.g. /etc/karmada/pki/etcd-client/ca.crt
    • /etc/karmada/pki/<server>-client/tls.crt // e.g. /etc/karmada/pki/etcd-client/tls.crt
    • /etc/karmada/pki/<server>-client/tls.key // e.g. /etc/karmada/pki/etcd-client/tls.key

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will make the changes.

// CaCertDataKey defines the data key for a CA cert
CaCertDataKey = "ca.crt"
// TLSCertDataKey defines the data key for a TLS cert
TLSCertDataKey = "tls.crt"
// TLSPrivateKeyDataKey defines the data key for a TLS cert private key
TLSPrivateKeyDataKey = "tls.key"

// CertificateValidity Certificate validity period
CertificateValidity = time.Hour * 24 * 365
Expand Down Expand Up @@ -125,6 +135,9 @@ const (

// APIServiceName defines the karmada aggregated apiserver APIService resource name.
APIServiceName = "v1alpha1.cluster.karmada.io"

// KarmadaOperatorEtcdClientCertNameSuffix defines the suffix for Karmada operator etcd client cert secret name
KarmadaOperatorEtcdClientCertNameSuffix = "karmada-operator-etcd-client-cert"
)

var (
Expand Down
38 changes: 38 additions & 0 deletions operator/pkg/controller/karmada/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@ package karmada

import (
"context"
"fmt"
"reflect"
"strconv"
"time"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/record"
Expand All @@ -36,6 +40,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/predicate"

operatorv1alpha1 "github.com/karmada-io/karmada/operator/pkg/apis/operator/v1alpha1"
"github.com/karmada-io/karmada/operator/pkg/constants"
operatorscheme "github.com/karmada-io/karmada/operator/pkg/scheme"
)

Expand All @@ -48,6 +53,9 @@ const (

// DisableCascadingDeletionLabel is the label that determine whether to perform cascade deletion
DisableCascadingDeletionLabel = "operator.karmada.io/disable-cascading-deletion"

// InvalidExternalEtcdClientSecretName is the reason for an invalid external etcd client secret name
InvalidExternalEtcdClientSecretName = "InvalidExternalEtcdClientSecretName"
)

// Controller controls the Karmada resource.
Expand Down Expand Up @@ -77,6 +85,11 @@ func (ctrl *Controller) Reconcile(ctx context.Context, req controllerruntime.Req
return controllerruntime.Result{}, err
}

if err := ctrl.validateKarmada(karmada); err != nil {
klog.Error(err, "Validation failed for karmada")
return controllerruntime.Result{}, nil
}

// The object is being deleted
if !karmada.DeletionTimestamp.IsZero() {
val, ok := karmada.Labels[DisableCascadingDeletionLabel]
Expand All @@ -96,6 +109,31 @@ func (ctrl *Controller) Reconcile(ctx context.Context, req controllerruntime.Req
return controllerruntime.Result{}, ctrl.syncKarmada(karmada)
}

// validateKarmada ensures the Karmada resource adheres to validation rules
func (ctrl *Controller) validateKarmada(karmada *operatorv1alpha1.Karmada) error {
if karmada.Spec.Components.Etcd != nil && karmada.Spec.Components.Etcd.External != nil {
expectedSecretName := fmt.Sprintf("%s-%s", karmada.Name, constants.KarmadaOperatorEtcdClientCertNameSuffix)
Copy link
Member

Choose a reason for hiding this comment

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

Can you remind me of the benefit of using the karmada instance name(karma.name) as the prefix?

In my opinion, each karmada instance would run in a separate namespace, so it does not necessarily have the prefix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explore further.

if karmada.Spec.Components.Etcd.External.SecretRef.Name != expectedSecretName {
errorMessage := fmt.Sprintf("Secret name for external etcd client must be %s, but got %s", expectedSecretName, karmada.Spec.Components.Etcd.External.SecretRef.Name)
ctrl.EventRecorder.Event(karmada, corev1.EventTypeWarning, InvalidExternalEtcdClientSecretName, errorMessage)

newCondition := metav1.Condition{
Type: string(operatorv1alpha1.Ready),
Status: metav1.ConditionFalse,
Reason: InvalidExternalEtcdClientSecretName,
Message: errorMessage,
LastTransitionTime: metav1.Now(),
}
meta.SetStatusCondition(&karmada.Status.Conditions, newCondition)
Comment on lines +120 to +127
Copy link
Member

Choose a reason for hiding this comment

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

I'd hesitate to introduce a condition for each concrete unexpected situation. My concern is we probably will have a lot of validations, we can't introduce a new condition for each validation.

I didn't expect to have condition from this PR, but thanks for me :) Maybe we can have a condition to represent the overall validation result, and we can put the error message to the Message if validation fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will create just one generic validation reason and use the message to describe the error.

if err := ctrl.Status().Update(context.TODO(), karmada); err != nil {
return err
}
return fmt.Errorf(errorMessage)
}
}
return nil
}

func (ctrl *Controller) syncKarmada(karmada *operatorv1alpha1.Karmada) error {
klog.V(2).InfoS("Reconciling karmada", "name", karmada.Name)
planner, err := NewPlannerFor(karmada, ctrl.Client, ctrl.Config)
Expand Down
69 changes: 36 additions & 33 deletions operator/pkg/controlplane/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ import (
clientsetscheme "k8s.io/client-go/kubernetes/scheme"

operatorv1alpha1 "github.com/karmada-io/karmada/operator/pkg/apis/operator/v1alpha1"
"github.com/karmada-io/karmada/operator/pkg/constants"
"github.com/karmada-io/karmada/operator/pkg/controlplane/etcd"
"github.com/karmada-io/karmada/operator/pkg/util"
"github.com/karmada-io/karmada/operator/pkg/util/apiclient"
"github.com/karmada-io/karmada/operator/pkg/util/patcher"
)

// EnsureKarmadaAPIServer creates karmada apiserver deployment and service resource
func EnsureKarmadaAPIServer(client clientset.Interface, cfg *operatorv1alpha1.KarmadaComponents, name, namespace string, featureGates map[string]bool) error {
if err := installKarmadaAPIServer(client, cfg.KarmadaAPIServer, name, namespace, featureGates); err != nil {
if err := installKarmadaAPIServer(client, cfg.KarmadaAPIServer, cfg.Etcd, name, namespace, featureGates); err != nil {
return fmt.Errorf("failed to install karmada apiserver, err: %w", err)
}

Expand All @@ -44,29 +44,25 @@ func EnsureKarmadaAPIServer(client clientset.Interface, cfg *operatorv1alpha1.Ka

// EnsureKarmadaAggregatedAPIServer creates karmada aggregated apiserver deployment and service resource
func EnsureKarmadaAggregatedAPIServer(client clientset.Interface, cfg *operatorv1alpha1.KarmadaComponents, name, namespace string, featureGates map[string]bool) error {
if err := installKarmadaAggregatedAPIServer(client, cfg.KarmadaAggregatedAPIServer, name, namespace, featureGates); err != nil {
if err := installKarmadaAggregatedAPIServer(client, cfg.KarmadaAggregatedAPIServer, cfg.Etcd, name, namespace, featureGates); err != nil {
return err
}
return createKarmadaAggregatedAPIServerService(client, name, namespace)
}

func installKarmadaAPIServer(client clientset.Interface, cfg *operatorv1alpha1.KarmadaAPIServer, name, namespace string, _ map[string]bool) error {
func installKarmadaAPIServer(client clientset.Interface, cfg *operatorv1alpha1.KarmadaAPIServer, etcdCfg *operatorv1alpha1.Etcd, name, namespace string, _ map[string]bool) error {
apiserverDeploymentBytes, err := util.ParseTemplate(KarmadaApiserverDeployment, struct {
DeploymentName, Namespace, Image, ImagePullPolicy, EtcdClientService string
ServiceSubnet, KarmadaCertsSecret, EtcdCertsSecret string
Replicas *int32
EtcdListenClientPort int32
DeploymentName, Namespace, Image, ImagePullPolicy string
ServiceSubnet, KarmadaCertsSecret string
Replicas *int32
}{
DeploymentName: util.KarmadaAPIServerName(name),
Namespace: namespace,
Image: cfg.Image.Name(),
ImagePullPolicy: string(cfg.ImagePullPolicy),
EtcdClientService: util.KarmadaEtcdClientName(name),
ServiceSubnet: *cfg.ServiceSubnet,
KarmadaCertsSecret: util.KarmadaCertSecretName(name),
EtcdCertsSecret: util.EtcdCertSecretName(name),
Replicas: cfg.Replicas,
EtcdListenClientPort: constants.EtcdListenClientPort,
DeploymentName: util.KarmadaAPIServerName(name),
Namespace: namespace,
Image: cfg.Image.Name(),
ImagePullPolicy: string(cfg.ImagePullPolicy),
ServiceSubnet: *cfg.ServiceSubnet,
KarmadaCertsSecret: util.KarmadaCertSecretName(name),
Replicas: cfg.Replicas,
})
if err != nil {
return fmt.Errorf("error when parsing karmadaApiserver deployment template: %w", err)
Expand All @@ -76,6 +72,12 @@ func installKarmadaAPIServer(client clientset.Interface, cfg *operatorv1alpha1.K
if err := kuberuntime.DecodeInto(clientsetscheme.Codecs.UniversalDecoder(), apiserverDeploymentBytes, apiserverDeployment); err != nil {
return fmt.Errorf("error when decoding karmadaApiserver deployment: %w", err)
}

err = etcd.ConfigureClientCredentials(apiserverDeployment, etcdCfg, name, namespace)
if err != nil {
return err
}

patcher.NewPatcher().WithAnnotations(cfg.Annotations).WithLabels(cfg.Labels).
WithExtraArgs(cfg.ExtraArgs).WithExtraVolumeMounts(cfg.ExtraVolumeMounts).
WithExtraVolumes(cfg.ExtraVolumes).WithResources(cfg.Resources).ForDeployment(apiserverDeployment)
Expand Down Expand Up @@ -112,23 +114,19 @@ func createKarmadaAPIServerService(client clientset.Interface, cfg *operatorv1al
return nil
}

func installKarmadaAggregatedAPIServer(client clientset.Interface, cfg *operatorv1alpha1.KarmadaAggregatedAPIServer, name, namespace string, featureGates map[string]bool) error {
func installKarmadaAggregatedAPIServer(client clientset.Interface, cfg *operatorv1alpha1.KarmadaAggregatedAPIServer, etcdCfg *operatorv1alpha1.Etcd, name, namespace string, featureGates map[string]bool) error {
aggregatedAPIServerDeploymentBytes, err := util.ParseTemplate(KarmadaAggregatedAPIServerDeployment, struct {
DeploymentName, Namespace, Image, ImagePullPolicy, EtcdClientService string
KubeconfigSecret, KarmadaCertsSecret, EtcdCertsSecret string
Replicas *int32
EtcdListenClientPort int32
DeploymentName, Namespace, Image, ImagePullPolicy string
KubeconfigSecret, KarmadaCertsSecret string
Replicas *int32
}{
DeploymentName: util.KarmadaAggregatedAPIServerName(name),
Namespace: namespace,
Image: cfg.Image.Name(),
ImagePullPolicy: string(cfg.ImagePullPolicy),
EtcdClientService: util.KarmadaEtcdClientName(name),
KubeconfigSecret: util.AdminKubeconfigSecretName(name),
KarmadaCertsSecret: util.KarmadaCertSecretName(name),
EtcdCertsSecret: util.EtcdCertSecretName(name),
Replicas: cfg.Replicas,
EtcdListenClientPort: constants.EtcdListenClientPort,
DeploymentName: util.KarmadaAggregatedAPIServerName(name),
Namespace: namespace,
Image: cfg.Image.Name(),
ImagePullPolicy: string(cfg.ImagePullPolicy),
KubeconfigSecret: util.AdminKubeconfigSecretName(name),
KarmadaCertsSecret: util.KarmadaCertSecretName(name),
Replicas: cfg.Replicas,
})
if err != nil {
return fmt.Errorf("error when parsing karmadaAggregatedAPIServer deployment template: %w", err)
Expand All @@ -139,6 +137,11 @@ func installKarmadaAggregatedAPIServer(client clientset.Interface, cfg *operator
return fmt.Errorf("err when decoding karmadaApiserver deployment: %w", err)
}

err = etcd.ConfigureClientCredentials(aggregatedAPIServerDeployment, etcdCfg, name, namespace)
if err != nil {
return err
}

patcher.NewPatcher().WithAnnotations(cfg.Annotations).WithLabels(cfg.Labels).
WithExtraArgs(cfg.ExtraArgs).WithFeatureGates(featureGates).WithResources(cfg.Resources).ForDeployment(aggregatedAPIServerDeployment)

Expand Down
Loading
Loading