From 57591621c75bb6941018f46f168df2fe2d74657e Mon Sep 17 00:00:00 2001 From: Johannes Frey Date: Mon, 23 Sep 2024 11:45:02 +0200 Subject: [PATCH] Make it possible to create vcluster with digits only name --- chart/templates/_helper.tpl | 8 +++++ chart/templates/headless-service.yaml | 2 +- chart/templates/service.yaml | 4 +-- chart/templates/statefulset.yaml | 10 +++--- chart/tests/headless-service_test.yaml | 10 ++++++ chart/tests/service_test.yaml | 10 ++++++ devspace.yaml | 7 ++-- pkg/certs/ensure.go | 4 +-- pkg/cli/connect_helm.go | 26 +++++++------- pkg/cli/find/find.go | 35 +++++++++++++++++++ pkg/config/parse.go | 13 ++++++- .../nodes/nodeservice/node_service.go | 2 +- pkg/setup/config.go | 1 + pkg/util/translate/types.go | 3 +- test/framework/framework.go | 14 +++++--- 15 files changed, 115 insertions(+), 34 deletions(-) diff --git a/chart/templates/_helper.tpl b/chart/templates/_helper.tpl index 680bbbbd0..fbc4ec9c6 100644 --- a/chart/templates/_helper.tpl +++ b/chart/templates/_helper.tpl @@ -15,3 +15,11 @@ {{ .repository }}:{{ .tag }} {{- end -}} {{- end -}} + +{{- define "vcluster.name" -}} +{{- if regexMatch "^[0-9]+$" .Release.Name -}} +{{- printf "vc-%s" .Release.Name -}} +{{- else -}} +{{- printf "%s" .Release.Name }} +{{- end -}} +{{- end -}} diff --git a/chart/templates/headless-service.yaml b/chart/templates/headless-service.yaml index 08df72eda..8af4d85e9 100644 --- a/chart/templates/headless-service.yaml +++ b/chart/templates/headless-service.yaml @@ -3,7 +3,7 @@ apiVersion: v1 kind: Service metadata: - name: {{ .Release.Name }}-headless + name: {{ template "vcluster.name" . }}-headless namespace: {{ .Release.Namespace }} labels: app: vcluster diff --git a/chart/templates/service.yaml b/chart/templates/service.yaml index 697297280..df46637d3 100644 --- a/chart/templates/service.yaml +++ b/chart/templates/service.yaml @@ -2,7 +2,7 @@ apiVersion: v1 kind: Service metadata: - name: {{ .Release.Name }} + name: {{ template "vcluster.name" . }} namespace: {{ .Release.Namespace }} labels: app: vcluster @@ -41,6 +41,6 @@ spec: {{- if and (not .Values.controlPlane.service.spec.selector) (not .Values.experimental.isolatedControlPlane.headless) }} selector: app: vcluster - release: {{ .Release.Name }} + release: {{ .Release.Name | quote }} {{- end }} {{- end }} diff --git a/chart/templates/statefulset.yaml b/chart/templates/statefulset.yaml index 910e8d829..6d975b752 100644 --- a/chart/templates/statefulset.yaml +++ b/chart/templates/statefulset.yaml @@ -2,7 +2,7 @@ apiVersion: apps/v1 kind: {{ include "vcluster.kind" . }} metadata: - name: {{ .Release.Name }} + name: {{ .Release.Name | quote }} namespace: {{ .Release.Namespace }} labels: app: vcluster @@ -21,13 +21,13 @@ spec: selector: matchLabels: app: vcluster - release: {{ .Release.Name }} + release: {{ .Release.Name | quote }} {{- if eq (include "vcluster.kind" .) "StatefulSet" }} {{- if ge (int .Capabilities.KubeVersion.Minor) 27 }} persistentVolumeClaimRetentionPolicy: whenDeleted: {{ .Values.controlPlane.statefulSet.persistence.volumeClaim.retentionPolicy }} {{- end }} - serviceName: {{ .Release.Name }}-headless + serviceName: {{ template "vcluster.name" . }}-headless podManagementPolicy: {{ .Values.controlPlane.statefulSet.scheduling.podManagementPolicy }} {{ include "vcluster.persistence" . | indent 2 }} {{- else }} @@ -51,7 +51,7 @@ spec: {{- end }} labels: app: vcluster - release: {{ .Release.Name }} + release: {{ .Release.Name | quote }} {{- if .Values.controlPlane.statefulSet.pods.labels }} {{ toYaml .Values.controlPlane.statefulSet.pods.labels | indent 8 }} {{- end }} @@ -87,7 +87,7 @@ spec: dnsPolicy: {{ .Values.controlPlane.statefulSet.dnsPolicy }} {{- end }} {{- if .Values.controlPlane.statefulSet.dnsConfig }} - dnsConfig: + dnsConfig: {{ toYaml .Values.controlPlane.statefulSet.dnsConfig | indent 8 }} {{- end }} {{- if .Values.controlPlane.advanced.serviceAccount.name }} diff --git a/chart/tests/headless-service_test.yaml b/chart/tests/headless-service_test.yaml index c0b8cbbf1..290c72626 100644 --- a/chart/tests/headless-service_test.yaml +++ b/chart/tests/headless-service_test.yaml @@ -61,6 +61,16 @@ tests: path: metadata.namespace value: my-namespace + - it: should prepend vc to service name when only digits are given + release: + name: 1234 + asserts: + - hasDocuments: + count: 1 + - equal: + path: metadata.name + value: vc-1234-headless + - it: embedded-etcd set: controlPlane: diff --git a/chart/tests/service_test.yaml b/chart/tests/service_test.yaml index cbbc7748b..b67bad554 100644 --- a/chart/tests/service_test.yaml +++ b/chart/tests/service_test.yaml @@ -93,6 +93,16 @@ tests: path: spec.ports count: 2 + - it: should prepend vc to service name when only digits are given + release: + name: 1234 + asserts: + - hasDocuments: + count: 1 + - equal: + path: metadata.name + value: vc-1234 + - it: isolated control plane release: name: my-release diff --git a/devspace.yaml b/devspace.yaml index ed8a590b8..346eadcad 100644 --- a/devspace.yaml +++ b/devspace.yaml @@ -9,6 +9,7 @@ vars: COMMON_VALUES: ./test/commonValues.yaml PRO_VALUES: ./test/proValues.yaml VALUES_FILE: ./test/e2e/values.yaml + RELEASE_NAME: "vcluster" # Images DevSpace will build for vcluster images: @@ -23,7 +24,7 @@ images: deployments: vcluster-k8s: helm: - releaseName: vcluster + releaseName: ${RELEASE_NAME} chart: name: ./chart values: @@ -51,7 +52,7 @@ deployments: vcluster-k3s: helm: - releaseName: vcluster + releaseName: ${RELEASE_NAME} chart: name: ./chart values: @@ -79,7 +80,7 @@ deployments: vcluster-k0s: helm: - releaseName: vcluster + releaseName: ${RELEASE_NAME} chart: name: ./chart values: diff --git a/pkg/certs/ensure.go b/pkg/certs/ensure.go index 01b44bbd2..3ab015ac5 100644 --- a/pkg/certs/ensure.go +++ b/pkg/certs/ensure.go @@ -107,9 +107,7 @@ func EnsureCerts( ownerRef := []metav1.OwnerReference{} if options.Experimental.SyncSettings.SetOwner { - // options.ServiceName gets rewritten to the workload service name so we use options.Name as the helm chart - // directly uses the release name for the service name - controlPlaneService, err := currentNamespaceClient.CoreV1().Services(currentNamespace).Get(ctx, options.Name, metav1.GetOptions{}) + controlPlaneService, err := currentNamespaceClient.CoreV1().Services(currentNamespace).Get(ctx, options.ControlPlaneService, metav1.GetOptions{}) if err != nil { return fmt.Errorf("get vcluster service: %w", err) } diff --git a/pkg/cli/connect_helm.go b/pkg/cli/connect_helm.go index 5443e10fb..b33a6fe5a 100644 --- a/pkg/cli/connect_helm.go +++ b/pkg/cli/connect_helm.go @@ -97,7 +97,7 @@ func (cmd *connectHelm) connect(ctx context.Context, vCluster *find.VCluster, co } // retrieve vcluster kube config - kubeConfig, err := cmd.getVClusterKubeConfig(ctx, vCluster.Name, command) + kubeConfig, err := cmd.getVClusterKubeConfig(ctx, vCluster, command) if err != nil { return err } @@ -272,7 +272,7 @@ func (cmd *connectHelm) prepare(ctx context.Context, vCluster *find.VCluster) er return nil } -func (cmd *connectHelm) getVClusterKubeConfig(ctx context.Context, vclusterName string, command []string) (*clientcmdapi.Config, error) { +func (cmd *connectHelm) getVClusterKubeConfig(ctx context.Context, vcluster *find.VCluster, command []string) (*clientcmdapi.Config, error) { var err error podName := cmd.PodName if podName == "" { @@ -280,7 +280,7 @@ func (cmd *connectHelm) getVClusterKubeConfig(ctx context.Context, vclusterName // get vcluster pod name var pods *corev1.PodList pods, err = cmd.kubeClient.CoreV1().Pods(cmd.Namespace).List(ctx, metav1.ListOptions{ - LabelSelector: "app=vcluster,release=" + vclusterName, + LabelSelector: "app=vcluster,release=" + vcluster.Name, }) if err != nil { return false, err @@ -310,7 +310,7 @@ func (cmd *connectHelm) getVClusterKubeConfig(ctx context.Context, vclusterName cmd.Log.Debugf("Successfully found vCluster pod for connecting %s", podName) // get the kube config from the Secret - kubeConfig, err := clihelper.GetKubeConfig(ctx, cmd.kubeClient, vclusterName, cmd.Namespace, cmd.Log) + kubeConfig, err := clihelper.GetKubeConfig(ctx, cmd.kubeClient, vcluster.Name, cmd.Namespace, cmd.Log) if err != nil { return nil, fmt.Errorf("failed to parse kube config: %w", err) } @@ -322,14 +322,14 @@ func (cmd *connectHelm) getVClusterKubeConfig(ctx context.Context, vclusterName } // exchange context name in virtual kube config - err = cmd.exchangeContextName(kubeConfig, vclusterName) + err = cmd.exchangeContextName(kubeConfig, vcluster.Name) if err != nil { return nil, err } // check if the vcluster is exposed and set server - if vclusterName != "" && cmd.Server == "" && len(command) == 0 { - err = cmd.setServerIfExposed(ctx, vclusterName, kubeConfig) + if vcluster.Name != "" && cmd.Server == "" && len(command) == 0 { + err = cmd.setServerIfExposed(ctx, vcluster, kubeConfig) if err != nil { return nil, err } @@ -338,7 +338,7 @@ func (cmd *connectHelm) getVClusterKubeConfig(ctx context.Context, vclusterName if cmd.Server == "" && cmd.BackgroundProxy { if localkubernetes.IsDockerInstalledAndUpAndRunning() { // start background container - server, err := localkubernetes.CreateBackgroundProxyContainer(ctx, vclusterName, cmd.Namespace, cmd.kubeClientConfig, kubeConfig, cmd.LocalPort, cmd.Log) + server, err := localkubernetes.CreateBackgroundProxyContainer(ctx, vcluster.Name, cmd.Namespace, cmd.kubeClientConfig, kubeConfig, cmd.LocalPort, cmd.Log) if err != nil { cmd.Log.Warnf("Error exposing local vcluster, will fallback to port-forwarding: %v", err) cmd.BackgroundProxy = false @@ -419,12 +419,12 @@ func (cmd *connectHelm) getVClusterKubeConfig(ctx context.Context, vclusterName return kubeConfig, nil } -func (cmd *connectHelm) setServerIfExposed(ctx context.Context, vClusterName string, vClusterConfig *clientcmdapi.Config) error { +func (cmd *connectHelm) setServerIfExposed(ctx context.Context, vcluster *find.VCluster, vClusterConfig *clientcmdapi.Config) error { printedWaiting := false err := wait.PollUntilContextTimeout(ctx, time.Second*2, time.Minute*5, true, func(ctx context.Context) (done bool, err error) { // first check for load balancer service, look for the other service if it's not there loadBalancerMissing := false - service, err := cmd.kubeClient.CoreV1().Services(cmd.Namespace).Get(ctx, vClusterName, metav1.GetOptions{}) + service, err := cmd.kubeClient.CoreV1().Services(cmd.Namespace).Get(ctx, vcluster.ServiceName, metav1.GetOptions{}) if err != nil { if kerrors.IsNotFound(err) { loadBalancerMissing = true @@ -433,7 +433,7 @@ func (cmd *connectHelm) setServerIfExposed(ctx context.Context, vClusterName str } } if loadBalancerMissing { - service, err = cmd.kubeClient.CoreV1().Services(cmd.Namespace).Get(ctx, vClusterName, metav1.GetOptions{}) + service, err = cmd.kubeClient.CoreV1().Services(cmd.Namespace).Get(ctx, vcluster.ServiceName, metav1.GetOptions{}) if kerrors.IsNotFound(err) { return true, nil } else if err != nil { @@ -446,7 +446,7 @@ func (cmd *connectHelm) setServerIfExposed(ctx context.Context, vClusterName str // not a load balancer? Then don't wait if service.Spec.Type == corev1.ServiceTypeNodePort { - server, err := localkubernetes.ExposeLocal(ctx, vClusterName, cmd.Namespace, &cmd.rawConfig, vClusterConfig, service, cmd.LocalPort, cmd.Log) + server, err := localkubernetes.ExposeLocal(ctx, vcluster.Name, cmd.Namespace, &cmd.rawConfig, vClusterConfig, service, cmd.LocalPort, cmd.Log) if err != nil { cmd.Log.Warnf("Error exposing local vcluster, will fallback to port-forwarding: %v", err) } @@ -476,7 +476,7 @@ func (cmd *connectHelm) setServerIfExposed(ctx context.Context, vClusterName str return false, nil } - cmd.Log.Infof("Using vcluster %s load balancer endpoint: %s", vClusterName, cmd.Server) + cmd.Log.Infof("Using vcluster %s load balancer endpoint: %s", vcluster.Name, cmd.Server) return true, nil }) if err != nil { diff --git a/pkg/cli/find/find.go b/pkg/cli/find/find.go index 4b843ff60..532d53c59 100644 --- a/pkg/cli/find/find.go +++ b/pkg/cli/find/find.go @@ -27,11 +27,14 @@ import ( const VirtualClusterSelector = "app=vcluster" +var digitsOnlyRegex = regexp.MustCompile("^[0-9]+$") + type VCluster struct { ClientFactory clientcmd.ClientConfig `json:"-"` Created metav1.Time Name string Namespace string + ServiceName string Annotations map[string]string Labels map[string]string Status Status @@ -399,6 +402,11 @@ func getVCluster(ctx context.Context, object client.Object, context, release str status = string(StatusUnknown) } + service, err := getService(ctx, client, kubeClientConfig, namespace, release) + if err != nil { + return VCluster{}, err + } + switch vclusterObject := object.(type) { case *appsv1.StatefulSet: for _, container := range vclusterObject.Spec.Template.Spec.Containers { @@ -425,6 +433,7 @@ func getVCluster(ctx context.Context, object client.Object, context, release str return VCluster{ Name: release, Namespace: namespace, + ServiceName: service.Name, Annotations: object.GetAnnotations(), Labels: object.GetLabels(), Status: Status(status), @@ -435,6 +444,32 @@ func getVCluster(ctx context.Context, object client.Object, context, release str }, nil } +func getService(ctx context.Context, client *kubernetes.Clientset, kubeClientConfig clientcmd.ClientConfig, namespace, name string) (*corev1.Service, error) { + ctx, cancel := context.WithTimeout(ctx, time.Second*30) + defer cancel() + + var svcName string + if digitsOnlyRegex.MatchString(name) { + svcName = fmt.Sprintf("vc-%s", name) + } else { + svcName = name + } + + service, err := client.CoreV1().Services(namespace).Get(ctx, svcName, metav1.GetOptions{}) + if err != nil { + if kerrors.IsForbidden(err) { + // try the current namespace instead + if namespace, err = getAccessibleNS(kubeClientConfig); err != nil { + return nil, err + } + return client.CoreV1().Services(namespace).Get(ctx, svcName, metav1.GetOptions{}) + } + return nil, err + } + + return service, nil +} + func getPods(ctx context.Context, client *kubernetes.Clientset, kubeClientConfig clientcmd.ClientConfig, namespace, podSelector string) (*corev1.PodList, error) { ctx, cancel := context.WithTimeout(ctx, time.Second*30) defer cancel() diff --git a/pkg/config/parse.go b/pkg/config/parse.go index c7c75ede3..a4adbc7c7 100644 --- a/pkg/config/parse.go +++ b/pkg/config/parse.go @@ -3,6 +3,7 @@ package config import ( "fmt" "os" + "regexp" "github.com/loft-sh/vcluster/config" "github.com/loft-sh/vcluster/pkg/strvals" @@ -10,6 +11,8 @@ import ( "sigs.k8s.io/yaml" ) +var digitsOnlyRegex = regexp.MustCompile("^[0-9]+$") + func ParseConfig(path, name string, setValues []string) (*VirtualClusterConfig, error) { // check if name is empty if name == "" { @@ -37,10 +40,18 @@ func ParseConfig(path, name string, setValues []string) (*VirtualClusterConfig, } // build config + + var svcName string + if digitsOnlyRegex.MatchString(name) { + svcName = fmt.Sprintf("vc-%s", name) + } else { + svcName = name + } + retConfig := &VirtualClusterConfig{ Config: *rawConfig, Name: name, - ControlPlaneService: name, + ControlPlaneService: svcName, } // validate config diff --git a/pkg/controllers/resources/nodes/nodeservice/node_service.go b/pkg/controllers/resources/nodes/nodeservice/node_service.go index 8d2436985..ca73c0945 100644 --- a/pkg/controllers/resources/nodes/nodeservice/node_service.go +++ b/pkg/controllers/resources/nodes/nodeservice/node_service.go @@ -123,7 +123,7 @@ func (n *nodeServiceProvider) Unlock() { } func (n *nodeServiceProvider) GetNodeIP(ctx context.Context, name string) (string, error) { - serviceName := translate.SafeConcatName(translate.VClusterName, "node", strings.ReplaceAll(name, ".", "-")) + serviceName := translate.SafeConcatName(translate.VClusterServiceName, "node", strings.ReplaceAll(name, ".", "-")) service := &corev1.Service{} err := n.currentNamespaceClient.Get(ctx, types.NamespacedName{Name: serviceName, Namespace: n.currentNamespace}, service) diff --git a/pkg/setup/config.go b/pkg/setup/config.go index ca1f510c4..fe0586a66 100644 --- a/pkg/setup/config.go +++ b/pkg/setup/config.go @@ -26,6 +26,7 @@ func InitAndValidateConfig(ctx context.Context, vConfig *config.VirtualClusterCo // set global vCluster name translate.VClusterName = vConfig.Name + translate.VClusterServiceName = vConfig.WorkloadService // set workload namespace err = os.Setenv("NAMESPACE", vConfig.WorkloadNamespace) diff --git a/pkg/util/translate/types.go b/pkg/util/translate/types.go index ab17d38ec..9a95b936c 100644 --- a/pkg/util/translate/types.go +++ b/pkg/util/translate/types.go @@ -25,7 +25,8 @@ var ( NamespaceLabelPrefix = "vcluster.loft.sh/ns-label" // VClusterName is the vcluster name, usually set at start time - VClusterName = "suffix" + VClusterName = "suffix" + VClusterServiceName = "vcluster" ManagedAnnotationsAnnotation = "vcluster.loft.sh/managed-annotations" ManagedLabelsAnnotation = "vcluster.loft.sh/managed-labels" diff --git a/test/framework/framework.go b/test/framework/framework.go index 526edfecb..af084b43e 100644 --- a/test/framework/framework.go +++ b/test/framework/framework.go @@ -24,10 +24,11 @@ import ( ) const ( - PollTimeout = time.Minute - DefaultVclusterName = "vcluster" - DefaultVclusterNamespace = "vcluster" - DefaultClientTimeout = 32 * time.Second // the default in client-go is 32 + PollTimeout = time.Minute + DefaultVclusterName = "vcluster" + DefaultVclusterNamespace = "vcluster" + DefaultVclusterServiceName = "vcluster" + DefaultClientTimeout = 32 * time.Second // the default in client-go is 32 ) var DefaultFramework = &Framework{} @@ -101,6 +102,10 @@ func CreateFramework(ctx context.Context, scheme *runtime.Scheme) error { if ns == "" { ns = DefaultVclusterNamespace } + serviceName := os.Getenv("VCLUSTER_SERVICE_NAME") + if serviceName == "" { + serviceName = DefaultVclusterServiceName + } timeoutEnvVar := os.Getenv("VCLUSTER_CLIENT_TIMEOUT") var timeout time.Duration timeoutInt, err := strconv.Atoi(timeoutEnvVar) @@ -116,6 +121,7 @@ func CreateFramework(ctx context.Context, scheme *runtime.Scheme) error { suffix = "vcluster" } translate.VClusterName = suffix + translate.VClusterServiceName = serviceName var multiNamespaceMode bool if os.Getenv("MULTINAMESPACE_MODE") == "true" {