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

add support for adopting external cluster #306

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
20 changes: 18 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ CONTAINER_REGISTRY ?= ghcr.io/kubestellar/kubeflex
# latest tag
LATEST_TAG ?= $(shell git describe --tags $(git rev-list --tags --max-count=1))

# Image URL to use all building/pushing image targets
IMG ?= ghcr.io/kubestellar/kubeflex/manager:latest
KO_DOCKER_REPO ?= ko.local
IMAGE_TAG ?= $(shell git rev-parse --short HEAD)
CMD_NAME ?= manager
IMG ?= ${KO_DOCKER_REPO}/${CMD_NAME}:${IMAGE_TAG}

# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
ENVTEST_K8S_VERSION = 1.26.1

Expand Down Expand Up @@ -195,6 +198,19 @@ chart: manifests kustomize
@mkdir -p chart/crds
$(KUSTOMIZE) build config/crd > chart/crds/crds.yaml

.PHONY: ko-local-build
ko-local-build:
KO_DOCKER_REPO=${KO_DOCKER_REPO} ko build -B ./cmd/${CMD_NAME} -t ${IMAGE_TAG} --platform linux/${ARCH}

# this is used for local testing
.PHONY: kind-load-image
kind-load-image:
kind load docker-image ${IMG} --name kubeflex

.PHONY: install-local-chart
install-local-chart: chart kind-load-image
helm upgrade --install --create-namespace -n kubeflex-system kubeflex-operator ./chart

##@ Build Dependencies

## Location to install dependencies to
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/conditions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,5 @@ func generateCondition(ctype ConditionType, reason ConditionReason, message stri
}

func addTime(t time.Duration) metav1.Time {
return metav1.NewTime(time.Now().Add(2 * time.Hour))
return metav1.NewTime(time.Now().Add(t))
}
18 changes: 13 additions & 5 deletions api/v1alpha1/controlplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,17 @@ import (

// ControlPlaneSpec defines the desired state of ControlPlane
type ControlPlaneSpec struct {
Type ControlPlaneType `json:"type,omitempty"`
Backend BackendDBType `json:"backend,omitempty"`
PostCreateHook *string `json:"postCreateHook,omitempty"`
PostCreateHookVars map[string]string `json:"postCreateHookVars,omitempty"`
Type ControlPlaneType `json:"type,omitempty"`
Backend BackendDBType `json:"backend,omitempty"`
// BootstrapSecretRef contains a reference to the kubeconfig used to bootstrap adoption of
// an external cluster
// +optional
Copy link
Contributor

@MikeSpreitzer MikeSpreitzer Dec 24, 2024

Choose a reason for hiding this comment

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

This is an "inline union": some unconditional fields plus a discriminator (Type) that determines whether some of the other fields should be present or absent. FYI, these are frowned upon in the Kubernetes API design community. They prefer pure unions: a pure union struct has only a discriminator plus fields whose proper presence is determined by the discriminator.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are correct, but I am concerned about introducing a non-backward compatible change if we go with the pure union. Just to be clear, is something like the following what you have in mind for the "pure union"?

type OffClusterSpec struct {
    BootstrapSecretRef           *SecretReference
    AdoptedTokenExpirationSeconds *int64
}

type InClusterSpec struct {
    Backend                      BackendDBType
    PostCreateHook     *string
    PostCreateHookVars map[string]string
}

type ControlPlaneSpec struct {
    Type          ControlPlaneType      `json:"type,omitempty"`
    inCluster         * InClusterSpec    `json:"inCluster,omitempty"`
    offCluster         * OffClusterSpec    `json:"offCluster,omitempty"`
}

Copy link
Contributor

Choose a reason for hiding this comment

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

What you showed looks like a pure union.

BootstrapSecretRef *SecretReference `json:"bootstrapSecretRef,omitempty"`
// expiration time for token of adopted cluster
// +optional
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved
AdoptedTokenExpirationSeconds *int64 `json:"adoptedTokenExpirationSeconds,omitempty"`
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved
PostCreateHook *string `json:"postCreateHook,omitempty"`
PostCreateHookVars map[string]string `json:"postCreateHookVars,omitempty"`
}

// ControlPlaneStatus defines the observed state of ControlPlane
Expand Down Expand Up @@ -71,14 +78,15 @@ const (
BackendDBTypeDedicated BackendDBType = "dedicated"
)

// +kubebuilder:validation:Enum=k8s;ocm;vcluster;host
// +kubebuilder:validation:Enum=k8s;ocm;vcluster;host;external
type ControlPlaneType string

const (
ControlPlaneTypeK8S ControlPlaneType = "k8s"
ControlPlaneTypeOCM ControlPlaneType = "ocm"
ControlPlaneTypeVCluster ControlPlaneType = "vcluster"
ControlPlaneTypeHost ControlPlaneType = "host"
ControlPlaneTypeExternal ControlPlaneType = "external"
)

// We do not use ObjectReference as its use is discouraged in favor of a locally defined type.
Expand Down
10 changes: 10 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

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

32 changes: 32 additions & 0 deletions chart/crds/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,42 @@ spec:
spec:
description: ControlPlaneSpec defines the desired state of ControlPlane
properties:
adoptedTokenExpirationSeconds:
description: expiration time for token of adopted cluster
format: int64
type: integer
backend:
enum:
- shared
- dedicated
type: string
bootstrapSecretRef:
description: |-
BootstrapSecretRef contains a reference to the kubeconfig used to bootstrap adoption of
an external cluster
properties:
inClusterKey:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please clarify what is inClusterKey?

Perhaps I missed, but I did not see a documentation of what the bootstrap secret should look like. This is useful if somebody wants/needs to create it without using kflex.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You may see the defaults being used for the bootstrap secret in cmd/kflex/adopt/adopt.go. The name is defined in:

func GenerateBoostrapSecretName(cpName string) string {
	return fmt.Sprintf("%s-bootstrap", cpName)
}

namespace is SystemNamespace = "kubeflex-system" and key is KubeconfigSecretKeyInCluster = "kubeconfig-incluster"

Copy link
Contributor

Choose a reason for hiding this comment

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

To further clarify, inClusterKey is the name of the secret generated by kflex from the bootstrap secret... right?

Since you have a default, can you make it not required?

Copy link
Collaborator Author

@pdettori pdettori Dec 17, 2024

Choose a reason for hiding this comment

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

To further clarify, inClusterKey is the name of the secret generated by kflex from the bootstrap secret

No, inClusterKey is the name of the key used for the kubeconfig, which is kubeconfig-incluster. The name of the secret would be <cp-name>-bootstrap. You do not have to provide these values in the ControlPlane CR, they are optional.

Copy link
Contributor

@MikeSpreitzer MikeSpreitzer Dec 20, 2024

Choose a reason for hiding this comment

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

(A) Since the golang source re-uses the existing type SecretReference, regularity is implied here. It is a reference to the same sort of thing. Same schema for contents. (And, BTW, the expectation(s) on Secret contents should be documented in the comment on the SecretReference type.) I am not sure yet whether this usage really is regular.

(B) @pdettori, what do you mean by "You do not have to provide these values in the ControlPlane CR, they are optional"? Which values are optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@MikeSpreitzer - re. (B) - what I stated previously was not correct. The bootstrap secret must be provided in the secret reference. Please check "Creating a control plane of type external with the API" for an example ControlPlane CR with the bootstrapSecretRef.

description: Required
type: string
key:
description: Required
type: string
name:
description: |-
`name` is the name of the secret.
Required
type: string
namespace:
description: |-
`namespace` is the namespace of the secret.
Required
type: string
required:
- inClusterKey
- key
- name
- namespace
type: object
postCreateHook:
type: string
postCreateHookVars:
Expand All @@ -71,6 +102,7 @@ spec:
- ocm
- vcluster
- host
- external
type: string
type: object
status:
Expand Down
5 changes: 3 additions & 2 deletions chart/templates/operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ spec:
- --secure-listen-address=0.0.0.0:8443
- --upstream=http://127.0.0.1:8080/
- --logtostderr=true
- --v={{.Values.verbosity}}
- --v=0
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved
image: gcr.io/kubebuilder/kube-rbac-proxy:v0.13.1
name: kube-rbac-proxy
ports:
Expand All @@ -614,12 +614,13 @@ spec:
- --health-probe-bind-address=:8081
- --metrics-bind-address=127.0.0.1:8080
- --leader-elect
- --zap-log-level={{max (.Values.verbosity | default 2 | int) 1}}
env:
- name: HELM_CONFIG_HOME
value: /tmp
- name: HELM_CACHE_HOME
value: /tmp
image: ghcr.io/kubestellar/kubeflex/manager:latest
image: ko.local/manager:3c7e658
imagePullPolicy: IfNotPresent
livenessProbe:
httpGet:
Expand Down
224 changes: 224 additions & 0 deletions cmd/kflex/adopt/adopt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
/*
Copyright 2024 The KubeStellar 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 adopt

import (
"context"
"fmt"
"os"
"strings"
"sync"

"path/filepath"

homedir "github.com/mitchellh/go-homedir"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/tools/clientcmd/api"
"sigs.k8s.io/controller-runtime/pkg/client"

tenancyv1alpha1 "github.com/kubestellar/kubeflex/api/v1alpha1"
"github.com/kubestellar/kubeflex/cmd/kflex/common"
cont "github.com/kubestellar/kubeflex/cmd/kflex/ctx"
kfclient "github.com/kubestellar/kubeflex/pkg/client"
"github.com/kubestellar/kubeflex/pkg/util"
)

type CPAdopt struct {
common.CP
AdoptedKubeconfig string
AdoptedContext string
AdoptedURLOverride string
AdoptedTokenExpirationSeconds int
SkipURLOverride bool
}

// Adopt a control plane from another cluster
func (c *CPAdopt) Adopt(hook string, hookVars []string, chattyStatus bool) {
done := make(chan bool)
var wg sync.WaitGroup
cx := cont.CPCtx{}
cx.Context(chattyStatus, false, false, false)

controlPlaneType := tenancyv1alpha1.ControlPlaneTypeExternal
util.PrintStatus(fmt.Sprintf("Adopting control plane %s of type %s ...", c.Name, controlPlaneType), done, &wg, chattyStatus)
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved

adoptedKubeconfig := getAdoptedKubeconfig(c)
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved

clp, err := kfclient.GetClient(c.Kubeconfig)
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting kubeflex client: %v\n", err)
os.Exit(1)
}
cl := *clp
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved

clientsetp, err := kfclient.GetClientSet(c.Kubeconfig)
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting clientset: %v\n", err)
os.Exit(1)
}

if err := applyAdoptedBootstrapSecret(clientsetp, c.Name, adoptedKubeconfig, c.AdoptedContext, c.AdoptedURLOverride, c.SkipURLOverride); err != nil {
fmt.Fprintf(os.Stderr, "error creating adopted cluster kubeconfig: %v\n", err)
os.Exit(1)
}

cp := common.GenerateControlPlane(c.Name, string(controlPlaneType), "", hook, hookVars)

if err := cl.Create(context.TODO(), cp, &client.CreateOptions{}); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, all writes should supply a FieldManager in the XXXOptions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I prefer to leave this to a separate campaign/PR as there are many other places where there are writes so should probably be done consistently everywhere.

fmt.Fprintf(os.Stderr, "Error creating ControlPlane object: %v\n", err)
os.Exit(1)
}

done <- true
wg.Wait()
}

func applyAdoptedBootstrapSecret(clientset *kubernetes.Clientset, cpName, adoptedKubeconfig, contextName, adoptedURLOverride string, skipURLOverride bool) error {
// Load the kubeconfig from file
config, err := clientcmd.LoadFromFile(adoptedKubeconfig)
if err != nil {
return fmt.Errorf("failed to load kubeconfig file %s: %v", adoptedKubeconfig, err)
}

// Retrieve the specified context
context, exists := config.Contexts[contextName]
if !exists {
return fmt.Errorf("context %s not found in the kubeconfig", contextName)
}

// Retrieve the associated cluster
cluster, exists := config.Clusters[context.Cluster]
if !exists {
return fmt.Errorf("cluster %s not found for context %s", context.Cluster, contextName)
}

// Construct a new kubeConfig object
kubeConfig := api.NewConfig()

kubeConfig.Clusters[context.Cluster] = cluster

if !skipURLOverride {
// Determine the server endpoint
endpoint := adoptedURLOverride
if endpoint == "" {
endpoint = cluster.Server
if !isValidServerURL(endpoint) {
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("invalid server endpoint %s. Please provide a valid value with the `url-override` option", endpoint)
}
}
kubeConfig.Clusters[context.Cluster].Server = endpoint
}

if authInfo, exists := config.AuthInfos[context.AuthInfo]; exists {
kubeConfig.AuthInfos[contextName] = authInfo
} else {
return fmt.Errorf("authInfo %s not found for context %s", context.AuthInfo, contextName)
}

kubeConfig.Contexts[contextName] = &api.Context{
Cluster: context.Cluster,
AuthInfo: contextName,
}
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved
kubeConfig.CurrentContext = contextName

newKubeConfig, err := clientcmd.Write(*kubeConfig)
if err != nil {
return fmt.Errorf("failed to serialize the new kubeconfig: %v", err)
}

createOrUpdateSecret(clientset, cpName, newKubeConfig)

return nil
}

func createOrUpdateSecret(clientset *kubernetes.Clientset, cpName string, kubeconfig []byte) error {

// Define the kubeconfig secret
kubeConfigSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: util.GenerateBoostrapSecretName(cpName),
Namespace: util.SystemNamespace,
},
Type: corev1.SecretTypeOpaque,
Data: map[string][]byte{util.KubeconfigSecretKeyInCluster: kubeconfig},
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved
}

// Try to create the secret
if _, err := clientset.CoreV1().Secrets(util.SystemNamespace).Create(context.TODO(), kubeConfigSecret, metav1.CreateOptions{}); err != nil {
// Check if the error is because the secret already exists
if apierrors.IsAlreadyExists(err) {
// Retrieve the existing secret
existingSecret, getErr := clientset.CoreV1().Secrets(util.SystemNamespace).Get(context.TODO(), util.AdminConfSecret, metav1.GetOptions{})
if getErr != nil {
return fmt.Errorf("failed to fetch existing secret %s in namespace %s: %v", util.AdminConfSecret, util.SystemNamespace, getErr)
}

// Update the data of the existing secret
existingSecret.Data = kubeConfigSecret.Data

// Update the secret with new data
if _, updateErr := clientset.CoreV1().Secrets(util.SystemNamespace).Update(context.TODO(), existingSecret, metav1.UpdateOptions{}); updateErr != nil {
return fmt.Errorf("failed to update existing secret %s in namespace %s: %v", util.AdminConfSecret, util.SystemNamespace, updateErr)
}
} else {
return fmt.Errorf("failed to create secret %s in namespace %s: %v", util.AdminConfSecret, util.SystemNamespace, err)
}
}

return nil
}

// check if the current server URL in the adopted cluster kubeconfig is using
// a local address, which would not work in a container
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved
func isValidServerURL(serverURL string) bool {
localAddresses := []string{"127.0.0.1", "localhost", "::1"}
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved
for _, addr := range localAddresses {
if strings.Contains(serverURL, addr) {
return false
}
}
return true
}

func getAdoptedKubeconfig(c *CPAdopt) string {
if c.AdoptedKubeconfig != "" {
return c.AdoptedKubeconfig
}
if c.Kubeconfig != "" {
return c.Kubeconfig
}
return getKubeConfigFromEnv(c.Kubeconfig)
}

func getKubeConfigFromEnv(kubeconfig string) string {
MikeSpreitzer marked this conversation as resolved.
Show resolved Hide resolved
if kubeconfig == "" {
kubeconfig = os.Getenv("KUBECONFIG")
if kubeconfig == "" {
home, err := homedir.Dir()
if err != nil {
fmt.Fprintf(os.Stderr, "Error finding home directory: %v\n", err)
os.Exit(1)
}
kubeconfig = filepath.Join(home, ".kube", "config")
}
}
return kubeconfig
}
Loading
Loading