From 675a4b9839616b0ab437656a4db92a7f351a12bb Mon Sep 17 00:00:00 2001 From: ycyaoxdu Date: Tue, 29 Nov 2022 08:29:30 +0000 Subject: [PATCH 1/4] add preflight check for join command Signed-off-by: ycyaoxdu --- pkg/cmd/init/exec.go | 5 +-- pkg/cmd/init/preflight/checks.go | 42 ----------------------- pkg/cmd/join/exec.go | 17 ++++++++-- pkg/cmd/join/preflight/checks.go | 30 +++++++++++++++++ pkg/cmd/join/preflight/checks_test.go | 48 +++++++++++++++++++++++++++ pkg/helpers/preflight/interface.go | 47 ++++++++++++++++++++++++++ 6 files changed, 143 insertions(+), 46 deletions(-) create mode 100644 pkg/cmd/join/preflight/checks.go create mode 100644 pkg/cmd/join/preflight/checks_test.go create mode 100644 pkg/helpers/preflight/interface.go diff --git a/pkg/cmd/init/exec.go b/pkg/cmd/init/exec.go index 61d10ae39..6fa060a12 100644 --- a/pkg/cmd/init/exec.go +++ b/pkg/cmd/init/exec.go @@ -14,6 +14,7 @@ import ( "open-cluster-management.io/clusteradm/pkg/cmd/init/scenario" "open-cluster-management.io/clusteradm/pkg/helpers" clusteradmjson "open-cluster-management.io/clusteradm/pkg/helpers/json" + preflightinterface "open-cluster-management.io/clusteradm/pkg/helpers/preflight" version "open-cluster-management.io/clusteradm/pkg/helpers/version" helperwait "open-cluster-management.io/clusteradm/pkg/helpers/wait" ) @@ -55,8 +56,8 @@ func (o *Options) validate() error { if err != nil { return err } - if err := preflight.RunChecks( - []preflight.Checker{ + if err := preflightinterface.RunChecks( + []preflightinterface.Checker{ preflight.HubApiServerCheck{ ClusterCtx: o.ClusteradmFlags.Context, ConfigPath: "", // TODO(@Promacanthus)： user custom kubeconfig path from command line arguments. diff --git a/pkg/cmd/init/preflight/checks.go b/pkg/cmd/init/preflight/checks.go index 2130ef661..8e95f64ca 100644 --- a/pkg/cmd/init/preflight/checks.go +++ b/pkg/cmd/init/preflight/checks.go @@ -2,10 +2,7 @@ package preflight import ( - "bytes" "context" - "fmt" - "io" "net" "net/url" @@ -21,25 +18,6 @@ import ( var BootstrapConfigMap = "cluster-info" -type Error struct { - Msg string -} - -func (e Error) Error() string { - return fmt.Sprintf("[preflight] Some fatal errors occurred:\n%s", e.Msg) -} - -func (e *Error) Preflight() bool { - return true -} - -// Checker validates the state of the cluster to ensure -// clusteradm will be successfully as often as possible. -type Checker interface { - Check() (warnings, errorList []error) - Name() string -} - type HubApiServerCheck struct { ClusterCtx string // current-context in kubeconfig ConfigPath string // kubeconfig file path @@ -161,23 +139,3 @@ func createClusterInfo(client kubernetes.Interface, cluster *clientcmdapi.Cluste } return CreateOrUpdateConfigMap(client, clusterInfo) } - -// RunChecks runs each check, display it's check/errors, -// and once all are processed will exist if any errors occured. -func RunChecks(checks []Checker, ww io.Writer) error { - var errsBuffer bytes.Buffer - for _, check := range checks { - name := check.Name() - warnings, errs := check.Check() - for _, warning := range warnings { - _, _ = io.WriteString(ww, fmt.Sprintf("\t[WARNING %s]: %v\n", name, warning)) - } - for _, err := range errs { - _, _ = errsBuffer.WriteString(fmt.Sprintf("\t[ERROR %s]: %v\n", name, err.Error())) - } - } - if errsBuffer.Len() > 0 { - return &Error{Msg: errsBuffer.String()} - } - return nil -} diff --git a/pkg/cmd/join/exec.go b/pkg/cmd/join/exec.go index c835fcc67..ee1df2f52 100644 --- a/pkg/cmd/join/exec.go +++ b/pkg/cmd/join/exec.go @@ -4,6 +4,7 @@ package join import ( "context" "fmt" + "os" "sync/atomic" "time" @@ -21,8 +22,10 @@ import ( clientcmdapiv1 "k8s.io/client-go/tools/clientcmd/api/v1" "k8s.io/klog/v2" "k8s.io/kubectl/pkg/cmd/util" + "open-cluster-management.io/clusteradm/pkg/cmd/join/preflight" "open-cluster-management.io/clusteradm/pkg/cmd/join/scenario" "open-cluster-management.io/clusteradm/pkg/helpers" + preflightinterface "open-cluster-management.io/clusteradm/pkg/helpers/preflight" "open-cluster-management.io/clusteradm/pkg/helpers/printer" "open-cluster-management.io/clusteradm/pkg/helpers/version" "open-cluster-management.io/clusteradm/pkg/helpers/wait" @@ -92,6 +95,16 @@ func (o *Options) validate() error { return fmt.Errorf("registry should not be empty") } + // preflight check + if err := preflightinterface.RunChecks( + []preflightinterface.Checker{ + preflight.KlusterletApiserverCheck{ + KlusterletApiserver: o.values.Klusterlet.APIServer, + }, + }, os.Stderr); err != nil { + return err + } + return nil } @@ -226,7 +239,7 @@ func waitUntilRegistrationOperatorConditionIsTrue(f util.Factory, timeout int64) }) } -//Wait until the klusterlet condition available=true, or timeout in $timeout seconds +// Wait until the klusterlet condition available=true, or timeout in $timeout seconds func waitUntilKlusterletConditionIsTrue(f util.Factory, timeout int64) error { client, err := f.KubernetesClientSet() if err != nil { @@ -273,7 +286,7 @@ func waitUntilKlusterletConditionIsTrue(f util.Factory, timeout int64) error { ) } -//Create bootstrap with token but without CA +// Create bootstrap with token but without CA func (o *Options) createExternalBootstrapConfig() clientcmdapiv1.Config { return clientcmdapiv1.Config{ // Define a cluster stanza based on the bootstrap kubeconfig. diff --git a/pkg/cmd/join/preflight/checks.go b/pkg/cmd/join/preflight/checks.go new file mode 100644 index 000000000..beb269ba6 --- /dev/null +++ b/pkg/cmd/join/preflight/checks.go @@ -0,0 +1,30 @@ +// Copyright Contributors to the Open Cluster Management project +package preflight + +import ( + "strings" + + "github.com/pkg/errors" +) + +type KlusterletApiserverCheck struct { + KlusterletApiserver string +} + +func (c KlusterletApiserverCheck) Check() (warnings []error, errorList []error) { + if !validAPIHost(c.KlusterletApiserver) { + return nil, []error{errors.New("ConfigMap/cluster-info.data.kubeconfig.clusters[0].cluster.server field in namespace kube-public should start with http:// or https://, please edit it first")} + } + return nil, nil +} + +func (c KlusterletApiserverCheck) Name() string { + return "KlusterletApiserver check" +} + +func validAPIHost(host string) bool { + if strings.HasPrefix(host, "http://") || strings.HasPrefix(host, "https://") { + return true + } + return false +} diff --git a/pkg/cmd/join/preflight/checks_test.go b/pkg/cmd/join/preflight/checks_test.go new file mode 100644 index 000000000..f61163f29 --- /dev/null +++ b/pkg/cmd/join/preflight/checks_test.go @@ -0,0 +1,48 @@ +// Copyright Contributors to the Open Cluster Management project +package preflight + +import ( + "testing" + + "github.com/pkg/errors" + testinghelper "open-cluster-management.io/clusteradm/pkg/helpers/testing" +) + +func TestKlusterletApiHostCheck_Check(t *testing.T) { + type fields struct { + apihost string + } + tests := []struct { + name string + fields fields + wantWarnings []error + wantErrorList []error + }{ + { + name: "invalid host", + fields: fields{ + apihost: "1.2.3.4:5678", + }, + wantWarnings: nil, + wantErrorList: []error{errors.New("ConfigMap/cluster-info.data.kubeconfig.clusters[0].cluster.server field in namespace kube-public should start with http:// or https://, please edit it first")}, + }, + { + name: "valid host", + fields: fields{ + apihost: "https://1.2.3.4:5678", + }, + wantWarnings: nil, + wantErrorList: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := KlusterletApiserverCheck{ + KlusterletApiserver: tt.fields.apihost, + } + gotWarnings, gotErrorList := c.Check() + testinghelper.AssertErrors(t, gotWarnings, tt.wantWarnings) + testinghelper.AssertErrors(t, gotErrorList, tt.wantErrorList) + }) + } +} diff --git a/pkg/helpers/preflight/interface.go b/pkg/helpers/preflight/interface.go new file mode 100644 index 000000000..07f0bb498 --- /dev/null +++ b/pkg/helpers/preflight/interface.go @@ -0,0 +1,47 @@ +// Copyright Contributors to the Open Cluster Management project +package preflight + +import ( + "bytes" + "fmt" + "io" +) + +// Checker validates the state of the cluster to ensure +// clusteradm will be successfully as often as possible. +type Checker interface { + Check() (warnings, errorList []error) + Name() string +} + +type Error struct { + Msg string +} + +func (e Error) Error() string { + return fmt.Sprintf("[preflight] Some fatal errors occurred:\n%s", e.Msg) +} + +func (e *Error) Preflight() bool { + return true +} + +// RunChecks runs each check, display it's check/errors, +// and once all are processed will exist if any errors occured. +func RunChecks(checks []Checker, ww io.Writer) error { + var errsBuffer bytes.Buffer + for _, check := range checks { + name := check.Name() + warnings, errs := check.Check() + for _, warning := range warnings { + _, _ = io.WriteString(ww, fmt.Sprintf("\t[WARNING %s]: %v\n", name, warning)) + } + for _, err := range errs { + _, _ = errsBuffer.WriteString(fmt.Sprintf("\t[ERROR %s]: %v\n", name, err.Error())) + } + } + if errsBuffer.Len() > 0 { + return &Error{Msg: errsBuffer.String()} + } + return nil +} From aa0ff2661c26db871cb35767349a08e1fc7c359f Mon Sep 17 00:00:00 2001 From: ycyaoxdu Date: Fri, 9 Dec 2022 07:43:26 +0000 Subject: [PATCH 2/4] refactor join process Signed-off-by: ycyaoxdu --- pkg/cmd/join/cmd.go | 6 +- pkg/cmd/join/exec.go | 150 ++++++++++-------- pkg/cmd/join/options.go | 39 +++-- pkg/cmd/join/preflight/checks.go | 45 +++++- .../join/scenario/join/klusterlets.cr.yaml | 4 +- pkg/cmd/join/scenario/join/operator.yaml | 2 +- pkg/helpers/client.go | 88 ++++++++-- 7 files changed, 227 insertions(+), 107 deletions(-) diff --git a/pkg/cmd/join/cmd.go b/pkg/cmd/join/cmd.go index ceaaef50d..0c287ae61 100644 --- a/pkg/cmd/join/cmd.go +++ b/pkg/cmd/join/cmd.go @@ -4,11 +4,10 @@ package join import ( "fmt" - genericclioptionsclusteradm "open-cluster-management.io/clusteradm/pkg/genericclioptions" - "open-cluster-management.io/clusteradm/pkg/helpers" - "github.com/spf13/cobra" "k8s.io/cli-runtime/pkg/genericclioptions" + genericclioptionsclusteradm "open-cluster-management.io/clusteradm/pkg/genericclioptions" + "open-cluster-management.io/clusteradm/pkg/helpers" ) var example = ` @@ -46,6 +45,7 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream cmd.Flags().StringVar(&o.token, "hub-token", "", "The token to access the hub") cmd.Flags().StringVar(&o.hubAPIServer, "hub-apiserver", "", "The api server url to the hub") + cmd.Flags().StringVar(&o.caFile, "ca-file", "", "the file path to hub ca, optional") cmd.Flags().StringVar(&o.clusterName, "cluster-name", "", "The name of the joining cluster") cmd.Flags().StringVar(&o.outputFile, "output-file", "", "The generated resources will be copied in the specified file") cmd.Flags().StringVar(&o.registry, "image-registry", "quay.io/open-cluster-management", "The name of the image registry serving OCM images.") diff --git a/pkg/cmd/join/exec.go b/pkg/cmd/join/exec.go index ee1df2f52..482585a8f 100644 --- a/pkg/cmd/join/exec.go +++ b/pkg/cmd/join/exec.go @@ -18,7 +18,6 @@ import ( "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" clientcmdapiv1 "k8s.io/client-go/tools/clientcmd/api/v1" "k8s.io/klog/v2" "k8s.io/kubectl/pkg/cmd/util" @@ -32,14 +31,26 @@ import ( ) func (o *Options) complete(cmd *cobra.Command, args []string) (err error) { + if o.token == "" { + return fmt.Errorf("token is missing") + } + if o.hubAPIServer == "" { + return fmt.Errorf("hub-server is missing") + } + if o.clusterName == "" { + return fmt.Errorf("name is missing") + } + if len(o.registry) == 0 { + return fmt.Errorf("the OCM image registry should not be empty, like quay.io/open-cluster-management") + } klog.V(1).InfoS("join options:", "dry-run", o.ClusteradmFlags.DryRun, "cluster", o.clusterName, "api-server", o.hubAPIServer, "output", o.outputFile) o.values = Values{ ClusterName: o.clusterName, Hub: Hub{ APIServer: o.hubAPIServer, - Registry: o.registry, }, + Registry: o.registry, } versionBundle, err := version.GetVersionBundle(o.bundleVersion) @@ -61,6 +72,34 @@ func (o *Options) complete(cmd *cobra.Command, args []string) (err error) { "'work image version'", versionBundle.Work, "'operator image version'", versionBundle.Operator) + // if --ca-file is set, read ca data + if o.caFile != "" { + cabytes, err := os.ReadFile(o.caFile) + if err != nil { + return err + } + o.HubCADate = cabytes + } + + // code logic of building hub client in join process: + // 1. use the token and insecure to fetch the ca data from cm in kube-public ns + // 2. if not found, assume using a authorized ca. + // 3. use the ca and token to build a secured client and call hub + + //Create an unsecure bootstrap + bootstrapExternalConfigUnSecure := o.createExternalBootstrapConfig() + //create external client from the bootstrap + externalClientUnSecure, err := helpers.CreateClientFromClientcmdapiv1Config(bootstrapExternalConfigUnSecure) + if err != nil { + return err + } + //Create the kubeconfig for the internal client + o.HubConfig, err = o.createClientcmdapiv1Config(externalClientUnSecure, bootstrapExternalConfigUnSecure) + if err != nil { + return err + } + + // get managed cluster externalServerURL kubeClient, err := o.ClusteradmFlags.KubectlFactory.KubernetesClientSet() if err != nil { klog.Errorf("Failed building kube client: %v", err) @@ -82,22 +121,12 @@ func (o *Options) complete(cmd *cobra.Command, args []string) (err error) { } func (o *Options) validate() error { - if o.token == "" { - return fmt.Errorf("token is missing") - } - if o.values.Hub.APIServer == "" { - return fmt.Errorf("hub-server is missing") - } - if o.values.ClusterName == "" { - return fmt.Errorf("name is missing") - } - if len(o.registry) == 0 { - return fmt.Errorf("registry should not be empty") - } - // preflight check if err := preflightinterface.RunChecks( []preflightinterface.Checker{ + preflight.HubKubeconfigCheck{ + Config: o.HubConfig, + }, preflight.KlusterletApiserverCheck{ KlusterletApiserver: o.values.Klusterlet.APIServer, }, @@ -105,6 +134,10 @@ func (o *Options) validate() error { return err } + err := o.setKubeconfig() + if err != nil { + return err + } return nil } @@ -112,21 +145,6 @@ func (o *Options) run() error { output := make([]string, 0) reader := scenario.GetScenarioResourcesReader() - //Create an unsecure bootstrap - bootstrapExternalConfigUnSecure := o.createExternalBootstrapConfig() - - //create external client from the bootstrap - externalClientUnSecure, err := createExternalClientFromBootstrap(bootstrapExternalConfigUnSecure) - if err != nil { - return err - } - - //Create the kubeconfig for the internal client - o.values.Hub.KubeConfig, err = o.createKubeConfig(externalClientUnSecure, bootstrapExternalConfigUnSecure) - if err != nil { - return err - } - kubeClient, apiExtensionsClient, dynamicClient, err := helpers.GetClients(o.ClusteradmFlags.KubectlFactory) if err != nil { return err @@ -323,53 +341,49 @@ func (o *Options) createExternalBootstrapConfig() clientcmdapiv1.Config { } } -func createExternalClientFromBootstrap(bootstrapExternalConfigUnSecure clientcmdapiv1.Config) (*kubernetes.Clientset, error) { - bootstrapConfigBytesUnSecure, err := yaml.Marshal(bootstrapExternalConfigUnSecure) - if err != nil { - return nil, err - } - - configUnSecure, err := clientcmd.Load(bootstrapConfigBytesUnSecure) - if err != nil { - return nil, err - } - restConfigUnSecure, err := clientcmd.NewDefaultClientConfig(*configUnSecure, &clientcmd.ConfigOverrides{}).ClientConfig() - if err != nil { - return nil, err - } - - clientUnSecure, err := kubernetes.NewForConfig(restConfigUnSecure) - if err != nil { - return nil, err - } - return clientUnSecure, nil -} - -func (o *Options) createKubeConfig(externalClientUnSecure *kubernetes.Clientset, - bootstrapExternalConfigUnSecure clientcmdapiv1.Config) (string, error) { - ca, err := helpers.GetCACert(externalClientUnSecure) - if err != nil { - return "", err - } - - hubApiserver := o.hubAPIServer - if o.forceHubInClusterEndpointLookup || len(hubApiserver) == 0 { - hubApiserver, err = helpers.GetAPIServer(externalClientUnSecure) +func (o *Options) createClientcmdapiv1Config(externalClientUnSecure *kubernetes.Clientset, + bootstrapExternalConfigUnSecure clientcmdapiv1.Config) (*clientcmdapiv1.Config, error) { + var err error + // set hub in cluster endpoint + if o.forceHubInClusterEndpointLookup { + o.hubInClusterEndpoint, err = helpers.GetAPIServer(externalClientUnSecure) if err != nil { if !errors.IsNotFound(err) { - return "", err + return nil, err } } } bootstrapConfig := bootstrapExternalConfigUnSecure.DeepCopy() bootstrapConfig.Clusters[0].Cluster.InsecureSkipTLSVerify = false - bootstrapConfig.Clusters[0].Cluster.CertificateAuthorityData = ca - bootstrapConfig.Clusters[0].Cluster.Server = hubApiserver - bootstrapConfigBytes, err := yaml.Marshal(bootstrapConfig) + bootstrapConfig.Clusters[0].Cluster.Server = o.hubAPIServer + if o.HubCADate != nil { + // directly set ca-data if --ca-file is set + bootstrapConfig.Clusters[0].Cluster.CertificateAuthorityData = o.HubCADate + } else { + // get ca data from externalClientUnsecure, ca may empty(cluster-info exists with no ca data) + ca, err := helpers.GetCACert(externalClientUnSecure) + if err != nil { + return nil, err + } + bootstrapConfig.Clusters[0].Cluster.CertificateAuthorityData = ca + } + + return bootstrapConfig, nil +} + +func (o *Options) setKubeconfig() error { + // replace apiserver if the flag is set, the apiserver value should not be set + // to in-cluster endpoint until preflight check is finished + if o.forceHubInClusterEndpointLookup { + o.HubConfig.Clusters[0].Cluster.Server = o.hubInClusterEndpoint + } + + bootstrapConfigBytes, err := yaml.Marshal(o.HubConfig) if err != nil { - return "", err + return err } - return string(bootstrapConfigBytes), nil + o.values.Hub.KubeConfig = string(bootstrapConfigBytes) + return nil } diff --git a/pkg/cmd/join/options.go b/pkg/cmd/join/options.go index dc5a7bb88..bf2b28698 100644 --- a/pkg/cmd/join/options.go +++ b/pkg/cmd/join/options.go @@ -3,59 +3,68 @@ package join import ( "k8s.io/cli-runtime/pkg/genericclioptions" + clientcmdapiv1 "k8s.io/client-go/tools/clientcmd/api/v1" genericclioptionsclusteradm "open-cluster-management.io/clusteradm/pkg/genericclioptions" ) -//Options: The structure holding all the command-line options +// Options: The structure holding all the command-line options type Options struct { //ClusteradmFlags: The generic options from the clusteradm cli-runtime. ClusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags + + //Values below are input from flags //The token generated on the hub to access it from the cluster token string //The external hub apiserver url (https://:) hubAPIServer string + //The hub ca-file(optional) + caFile string //the name under the cluster must be imported clusterName string - - values Values - //The file to output the resources will be sent to the file. - outputFile string - //version of predefined compatible image versions - bundleVersion string //Pulling image registry of OCM registry string + // version of predefined compatible image versions + bundleVersion string + //The file to output the resources will be sent to the file. + outputFile string //Runs the cluster joining in foreground wait bool - // By default, The installing registration agent will be starting registration using // the external endpoint from --hub-apiserver instead of looking for the internal // endpoint from the public cluster-info. forceHubInClusterEndpointLookup bool + hubInClusterEndpoint string + + //Values below are tempoary data + //HubCADate: data in hub ca file + HubCADate []byte + // hub config + HubConfig *clientcmdapiv1.Config + + //Values below are used to fill in yaml files + values Values } -//Values: The values used in the template +// Values: The values used in the template type Values struct { //ClusterName: the name of the joined cluster on the hub ClusterName string //Hub: Hub information Hub Hub - //Registry is the image registry related configuration - Registry string //Klusterlet is the klusterlet related configuration Klusterlet Klusterlet + //Registry is the image registry related configuration + Registry string //bundle version BundleVersion BundleVersion } -//Hub: The hub values for the template - +// Hub: The hub values for the template type Hub struct { //APIServer: The API Server external URL APIServer string //KubeConfig: The kubeconfig of the bootstrap secret to connect to the hub KubeConfig string - //image registry - Registry string } // Klusterlet is for templating klusterlet configuration diff --git a/pkg/cmd/join/preflight/checks.go b/pkg/cmd/join/preflight/checks.go index beb269ba6..b225bd66b 100644 --- a/pkg/cmd/join/preflight/checks.go +++ b/pkg/cmd/join/preflight/checks.go @@ -5,13 +5,15 @@ import ( "strings" "github.com/pkg/errors" + clientcmdapiv1 "k8s.io/client-go/tools/clientcmd/api/v1" + "open-cluster-management.io/clusteradm/pkg/helpers" ) type KlusterletApiserverCheck struct { KlusterletApiserver string } -func (c KlusterletApiserverCheck) Check() (warnings []error, errorList []error) { +func (c KlusterletApiserverCheck) Check() (warningList []error, errorList []error) { if !validAPIHost(c.KlusterletApiserver) { return nil, []error{errors.New("ConfigMap/cluster-info.data.kubeconfig.clusters[0].cluster.server field in namespace kube-public should start with http:// or https://, please edit it first")} } @@ -22,6 +24,47 @@ func (c KlusterletApiserverCheck) Name() string { return "KlusterletApiserver check" } +type HubKubeconfigCheck struct { + Config *clientcmdapiv1.Config +} + +func (c HubKubeconfigCheck) Check() (warningList []error, errorList []error) { + if c.Config == nil { + return nil, []error{errors.New("no hubconfig found")} + } + + if len(c.Config.Clusters) != 1 { + return nil, []error{errors.New("error cluster length")} + } + + // validate apiserver foramt + if !validAPIHost(c.Config.Clusters[0].Cluster.Server) { + return nil, []error{errors.New("--hub-apiserver should start with http:// or https://")} + } + // validate ca + if c.Config.Clusters[0].Cluster.CertificateAuthorityData == nil { + return []error{errors.New("no ca detected, creating hub kubeconfig without ca")}, nil + } + + // validate kubeconfig + discoveryClient, err := helpers.CreateDiscoveryClientFromClientcmdapiv1Config(*c.Config) + if err != nil { + return nil, []error{err} + } + + _, err = discoveryClient.ServerVersion() + if err != nil { + return nil, []error{err} + + } + return nil, nil +} + +func (c HubKubeconfigCheck) Name() string { + return "HubKubeconfig check" +} + +// utils func validAPIHost(host string) bool { if strings.HasPrefix(host, "http://") || strings.HasPrefix(host, "https://") { return true diff --git a/pkg/cmd/join/scenario/join/klusterlets.cr.yaml b/pkg/cmd/join/scenario/join/klusterlets.cr.yaml index f9140fed2..e59e62e8c 100644 --- a/pkg/cmd/join/scenario/join/klusterlets.cr.yaml +++ b/pkg/cmd/join/scenario/join/klusterlets.cr.yaml @@ -4,8 +4,8 @@ kind: Klusterlet metadata: name: klusterlet spec: - registrationImagePullSpec: {{ .Hub.Registry }}/registration:{{ .BundleVersion.RegistrationImageVersion }} - workImagePullSpec: {{ .Hub.Registry }}/work:{{ .BundleVersion.RegistrationImageVersion }} + registrationImagePullSpec: {{ .Registry }}/registration:{{ .BundleVersion.RegistrationImageVersion }} + workImagePullSpec: {{ .Registry }}/work:{{ .BundleVersion.RegistrationImageVersion }} clusterName: {{ .ClusterName }} namespace: open-cluster-management-agent externalServerURLs: diff --git a/pkg/cmd/join/scenario/join/operator.yaml b/pkg/cmd/join/scenario/join/operator.yaml index e69a0ce9b..3aa51059f 100644 --- a/pkg/cmd/join/scenario/join/operator.yaml +++ b/pkg/cmd/join/scenario/join/operator.yaml @@ -42,7 +42,7 @@ spec: serviceAccountName: klusterlet containers: - name: klusterlet - image: {{ .Hub.Registry }}/registration-operator:{{ .BundleVersion.RegistrationImageVersion }} + image: {{ .Registry }}/registration-operator:{{ .BundleVersion.RegistrationImageVersion }} args: - "/registration-operator" - "klusterlet" diff --git a/pkg/helpers/client.go b/pkg/helpers/client.go index cbd07ba23..1d78db635 100644 --- a/pkg/helpers/client.go +++ b/pkg/helpers/client.go @@ -21,9 +21,11 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" clientcmdapiv1 "k8s.io/client-go/tools/clientcmd/api/v1" "k8s.io/client-go/util/retry" "k8s.io/kubectl/pkg/cmd/util" @@ -65,7 +67,7 @@ func GetClients(f util.Factory) ( return } -//GetAPIServer gets the api server url +// GetAPIServer gets the api server url func GetAPIServer(kubeClient kubernetes.Interface) (string, error) { config, err := getClusterInfoKubeConfig(kubeClient) if err != nil { @@ -79,9 +81,10 @@ func GetAPIServer(kubeClient kubernetes.Interface) (string, error) { return cluster.Server, nil } -//GetCACert returns the CA cert. -//First by looking in the cluster-info configmap of the kube-public ns and if not found, -//it searches in the kube-root-ca.crt configmap. +// GetCACert returns the CA cert. +// First by looking in the cluster-info configmap of the kube-public ns and if not found, +// it secondly searches in the kube-root-ca.crt configmap of the kube-public ns and if +// also not found, it finally return empty value with no error. func GetCACert(kubeClient kubernetes.Interface) ([]byte, error) { config, err := getClusterInfoKubeConfig(kubeClient) if err == nil { @@ -94,10 +97,13 @@ func GetCACert(kubeClient kubernetes.Interface) ([]byte, error) { } if errors.IsNotFound(err) { cm, err := kubeClient.CoreV1().ConfigMaps("kube-public").Get(context.TODO(), "kube-root-ca.crt", metav1.GetOptions{}) - if err != nil { - return nil, err + if err == nil { + return []byte(cm.Data["ca.crt"]), nil + } + if errors.IsNotFound(err) { + return nil, nil } - return []byte(cm.Data["ca.crt"]), nil + return nil, err } return nil, err } @@ -115,7 +121,7 @@ func getClusterInfoKubeConfig(kubeClient kubernetes.Interface) (*clientcmdapiv1. return config, nil } -//WaitCRDToBeReady waits if a crd is ready +// WaitCRDToBeReady waits if a crd is ready func WaitCRDToBeReady(apiExtensionsClient apiextensionsclient.Interface, name string, b wait.Backoff, wait bool) error { errGet := retry.OnError(b, func(err error) bool { if err != nil { @@ -142,9 +148,9 @@ func WaitCRDToBeReady(apiExtensionsClient apiextensionsclient.Interface, name st return errGet } -//GetToken returns the bootstrap token. -//It searches first for the service-account token and then if it is not found -//it looks for the bootstrap token in kube-system. +// GetToken returns the bootstrap token. +// It searches first for the service-account token and then if it is not found +// it looks for the bootstrap token in kube-system. func GetToken(ctx context.Context, kubeClient kubernetes.Interface) (string, TokenType, error) { token, err := GetBootstrapTokenFromSA(ctx, kubeClient) if err != nil { @@ -161,7 +167,7 @@ func GetToken(ctx context.Context, kubeClient kubernetes.Interface) (string, Tok return token, ServiceAccountToken, nil } -//GetBootstrapSecret returns the secret in kube-system +// GetBootstrapSecret returns the secret in kube-system func GetBootstrapSecret(ctx context.Context, kubeClient kubernetes.Interface) (*corev1.Secret, error) { var bootstrapSecret *corev1.Secret l, err := kubeClient.CoreV1(). @@ -185,7 +191,7 @@ func GetBootstrapSecret(ctx context.Context, kubeClient kubernetes.Interface) (* return bootstrapSecret, err } -//GetBootstrapToken returns the token in kube-system +// GetBootstrapToken returns the token in kube-system func GetBootstrapToken(ctx context.Context, kubeClient kubernetes.Interface) (string, error) { bootstrapSecret, err := GetBootstrapSecret(ctx, kubeClient) if err != nil { @@ -194,7 +200,7 @@ func GetBootstrapToken(ctx context.Context, kubeClient kubernetes.Interface) (st return fmt.Sprintf("%s.%s", string(bootstrapSecret.Data["token-id"]), string(bootstrapSecret.Data["token-secret"])), nil } -//GetBootstrapSecretFromSA retrieves the service-account token secret +// GetBootstrapSecretFromSA retrieves the service-account token secret func GetBootstrapTokenFromSA(ctx context.Context, kubeClient kubernetes.Interface) (string, error) { tr, err := kubeClient.CoreV1(). ServiceAccounts(config.OpenClusterManagementNamespace). @@ -210,8 +216,8 @@ func GetBootstrapTokenFromSA(ctx context.Context, kubeClient kubernetes.Interfac return tr.Status.Token, nil } -//IsClusterManagerInstalled checks if the hub is already initialized. -//It checks if the crd is already present to find out that the hub is already initialized. +// IsClusterManagerInstalled checks if the hub is already initialized. +// It checks if the crd is already present to find out that the hub is already initialized. func IsClusterManagerInstalled(apiExtensionsClient apiextensionsclient.Interface) (bool, error) { _, err := apiExtensionsClient.ApiextensionsV1(). CustomResourceDefinitions(). @@ -228,7 +234,7 @@ func IsClusterManagerInstalled(apiExtensionsClient apiextensionsclient.Interface } // IsKlusterlets checks if the Managed cluster is already initialized. -//It checks if the crd is already present to find out that the managed cluster is already initialized. +// It checks if the crd is already present to find out that the managed cluster is already initialized. func IsKlusterletsInstalled(apiExtensionsClient apiextensionsclient.Interface) (bool, error) { _, err := apiExtensionsClient.ApiextensionsV1(). CustomResourceDefinitions(). @@ -265,3 +271,51 @@ func WatchUntil( } return nil } + +// CreateRESTConfigFromClientcmdapiv1Config +func CreateRESTConfigFromClientcmdapiv1Config(clientcmdapiv1Config clientcmdapiv1.Config) (*rest.Config, error) { + clientcmdapiv1ConfigBytes, err := yaml.Marshal(clientcmdapiv1Config) + if err != nil { + return nil, err + } + + apiConfig, err := clientcmd.Load(clientcmdapiv1ConfigBytes) + if err != nil { + return nil, err + } + + restConfig, err := clientcmd.NewDefaultClientConfig(*apiConfig, &clientcmd.ConfigOverrides{}).ClientConfig() + if err != nil { + return nil, err + } + + return restConfig, nil +} + +// CreateClientFromClientcmdapiv1Config +func CreateClientFromClientcmdapiv1Config(clientcmdapiv1Config clientcmdapiv1.Config) (*kubernetes.Clientset, error) { + restConfig, err := CreateRESTConfigFromClientcmdapiv1Config(clientcmdapiv1Config) + if err != nil { + return nil, err + } + + kubeClient, err := kubernetes.NewForConfig(restConfig) + if err != nil { + return nil, err + } + return kubeClient, nil +} + +// CreateDiscoveryClientFromClientcmdapiv1Config +func CreateDiscoveryClientFromClientcmdapiv1Config(clientcmdapiv1Config clientcmdapiv1.Config) (*discovery.DiscoveryClient, error) { + restConfig, err := CreateRESTConfigFromClientcmdapiv1Config(clientcmdapiv1Config) + if err != nil { + return nil, err + } + + discoveryClient, err := discovery.NewDiscoveryClientForConfig(restConfig) + if err != nil { + return nil, err + } + return discoveryClient, nil +} From 770bff26f29cbc6c678ea10d08c1eb55697644e9 Mon Sep 17 00:00:00 2001 From: ycyaoxdu Date: Mon, 19 Dec 2022 07:46:37 +0000 Subject: [PATCH 3/4] error with no ca data Signed-off-by: ycyaoxdu --- pkg/cmd/join/preflight/checks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/join/preflight/checks.go b/pkg/cmd/join/preflight/checks.go index b225bd66b..08489630c 100644 --- a/pkg/cmd/join/preflight/checks.go +++ b/pkg/cmd/join/preflight/checks.go @@ -43,7 +43,7 @@ func (c HubKubeconfigCheck) Check() (warningList []error, errorList []error) { } // validate ca if c.Config.Clusters[0].Cluster.CertificateAuthorityData == nil { - return []error{errors.New("no ca detected, creating hub kubeconfig without ca")}, nil + return nil, []error{errors.New("no ca detected, creating hub kubeconfig without ca")} } // validate kubeconfig From ca2e3b81f91832a76aa33f882d3745a33e9f889b Mon Sep 17 00:00:00 2001 From: ycyaoxdu Date: Wed, 21 Dec 2022 07:41:44 +0000 Subject: [PATCH 4/4] move preflight warningList from []error to []string Signed-off-by: ycyaoxdu --- pkg/cmd/init/preflight/checks.go | 12 ++++++------ pkg/cmd/init/preflight/checks_test.go | 14 +++++++------- pkg/cmd/join/preflight/checks.go | 4 ++-- pkg/cmd/join/preflight/checks_test.go | 4 ++-- pkg/helpers/preflight/interface.go | 2 +- pkg/helpers/testing/assertion.go | 11 +++++++++++ 6 files changed, 29 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/init/preflight/checks.go b/pkg/cmd/init/preflight/checks.go index 8e95f64ca..07b6616d9 100644 --- a/pkg/cmd/init/preflight/checks.go +++ b/pkg/cmd/init/preflight/checks.go @@ -23,7 +23,7 @@ type HubApiServerCheck struct { ConfigPath string // kubeconfig file path } -func (c HubApiServerCheck) Check() (warnings []error, errorList []error) { +func (c HubApiServerCheck) Check() (warnings []string, errorList []error) { cluster, err := loadCurrentCluster(c.ClusterCtx, c.ConfigPath) if err != nil { return nil, []error{err} @@ -37,7 +37,7 @@ func (c HubApiServerCheck) Check() (warnings []error, errorList []error) { return nil, []error{err} } if net.ParseIP(host) == nil { - return []error{errors.New("Hub Api Server is a domain name, maybe you should set HostAlias in klusterlet")}, nil + return []string{"Hub Api Server is a domain name, maybe you should set HostAlias in klusterlet"}, nil } return nil, nil } @@ -55,19 +55,19 @@ type ClusterInfoCheck struct { Client kubernetes.Interface } -func (c ClusterInfoCheck) Check() (warnings []error, errorList []error) { +func (c ClusterInfoCheck) Check() (warnings []string, errorList []error) { cm, err := c.Client.CoreV1().ConfigMaps(c.Namespace).Get(context.Background(), c.ResourceName, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { resourceNotFound := errors.New("no ConfigMap named cluster-info in the kube-public namespace, clusteradm will creates it") cluster, err := loadCurrentCluster(c.ClusterCtx, c.ConfigPath) if err != nil { - return []error{resourceNotFound}, []error{err} + return []string{resourceNotFound.Error()}, []error{err} } if err := createClusterInfo(c.Client, cluster); err != nil { - return []error{resourceNotFound}, []error{err} + return []string{resourceNotFound.Error()}, []error{err} } - return []error{resourceNotFound}, nil + return []string{resourceNotFound.Error()}, nil } return nil, []error{err} } diff --git a/pkg/cmd/init/preflight/checks_test.go b/pkg/cmd/init/preflight/checks_test.go index 2025ad7aa..02b023db9 100644 --- a/pkg/cmd/init/preflight/checks_test.go +++ b/pkg/cmd/init/preflight/checks_test.go @@ -115,7 +115,7 @@ func TestHubApiServerCheck_Check(t *testing.T) { tests := []struct { name string fields fields - wantWarnings []error + wantWarnings []string wantErrorList []error }{ { @@ -124,7 +124,7 @@ func TestHubApiServerCheck_Check(t *testing.T) { ClusterCtx: "", ConfigPath: kubeconfigFilePath, }, - wantWarnings: []error{errors.New("Hub Api Server is a domain name, maybe you should set HostAlias in klusterlet")}, + wantWarnings: []string{"Hub Api Server is a domain name, maybe you should set HostAlias in klusterlet"}, wantErrorList: nil, }, { @@ -144,7 +144,7 @@ func TestHubApiServerCheck_Check(t *testing.T) { ClusterCtx: currentContext, ConfigPath: kubeconfigFilePath, }, - wantWarnings: []error{errors.New("Hub Api Server is a domain name, maybe you should set HostAlias in klusterlet")}, + wantWarnings: []string{"Hub Api Server is a domain name, maybe you should set HostAlias in klusterlet"}, wantErrorList: nil, }, { @@ -164,7 +164,7 @@ func TestHubApiServerCheck_Check(t *testing.T) { ConfigPath: tt.fields.ConfigPath, } gotWarnings, gotErrorList := c.Check() - testinghelper.AssertErrors(t, gotWarnings, tt.wantWarnings) + testinghelper.AssertWarnings(t, gotWarnings, tt.wantWarnings) testinghelper.AssertErrors(t, gotErrorList, tt.wantErrorList) }) } @@ -183,7 +183,7 @@ func TestClusterInfoCheck_Check(t *testing.T) { fields fields actionIndex int action string - wantWarnings []error + wantWarnings []string wantErrorList []error }{ { @@ -225,7 +225,7 @@ func TestClusterInfoCheck_Check(t *testing.T) { }, actionIndex: 1, action: "create", - wantWarnings: []error{errors.New("no ConfigMap named cluster-info in the kube-public namespace, clusteradm will creates it")}, + wantWarnings: []string{"no ConfigMap named cluster-info in the kube-public namespace, clusteradm will creates it"}, wantErrorList: nil, }, } @@ -242,7 +242,7 @@ func TestClusterInfoCheck_Check(t *testing.T) { } gotWarnings, gotErrorList := c.Check() testinghelper.AssertAction(t, client.Actions()[tt.actionIndex], tt.action) - testinghelper.AssertErrors(t, gotWarnings, tt.wantWarnings) + testinghelper.AssertWarnings(t, gotWarnings, tt.wantWarnings) testinghelper.AssertErrors(t, gotErrorList, tt.wantErrorList) }) } diff --git a/pkg/cmd/join/preflight/checks.go b/pkg/cmd/join/preflight/checks.go index 08489630c..e8a216ed1 100644 --- a/pkg/cmd/join/preflight/checks.go +++ b/pkg/cmd/join/preflight/checks.go @@ -13,7 +13,7 @@ type KlusterletApiserverCheck struct { KlusterletApiserver string } -func (c KlusterletApiserverCheck) Check() (warningList []error, errorList []error) { +func (c KlusterletApiserverCheck) Check() (warningList []string, errorList []error) { if !validAPIHost(c.KlusterletApiserver) { return nil, []error{errors.New("ConfigMap/cluster-info.data.kubeconfig.clusters[0].cluster.server field in namespace kube-public should start with http:// or https://, please edit it first")} } @@ -28,7 +28,7 @@ type HubKubeconfigCheck struct { Config *clientcmdapiv1.Config } -func (c HubKubeconfigCheck) Check() (warningList []error, errorList []error) { +func (c HubKubeconfigCheck) Check() (warningList []string, errorList []error) { if c.Config == nil { return nil, []error{errors.New("no hubconfig found")} } diff --git a/pkg/cmd/join/preflight/checks_test.go b/pkg/cmd/join/preflight/checks_test.go index f61163f29..fdf5b6880 100644 --- a/pkg/cmd/join/preflight/checks_test.go +++ b/pkg/cmd/join/preflight/checks_test.go @@ -15,7 +15,7 @@ func TestKlusterletApiHostCheck_Check(t *testing.T) { tests := []struct { name string fields fields - wantWarnings []error + wantWarnings []string wantErrorList []error }{ { @@ -41,7 +41,7 @@ func TestKlusterletApiHostCheck_Check(t *testing.T) { KlusterletApiserver: tt.fields.apihost, } gotWarnings, gotErrorList := c.Check() - testinghelper.AssertErrors(t, gotWarnings, tt.wantWarnings) + testinghelper.AssertWarnings(t, gotWarnings, tt.wantWarnings) testinghelper.AssertErrors(t, gotErrorList, tt.wantErrorList) }) } diff --git a/pkg/helpers/preflight/interface.go b/pkg/helpers/preflight/interface.go index 07f0bb498..e530f880d 100644 --- a/pkg/helpers/preflight/interface.go +++ b/pkg/helpers/preflight/interface.go @@ -10,7 +10,7 @@ import ( // Checker validates the state of the cluster to ensure // clusteradm will be successfully as often as possible. type Checker interface { - Check() (warnings, errorList []error) + Check() (warnings []string, errorList []error) Name() string } diff --git a/pkg/helpers/testing/assertion.go b/pkg/helpers/testing/assertion.go index 903c8ae17..e09dcbe91 100644 --- a/pkg/helpers/testing/assertion.go +++ b/pkg/helpers/testing/assertion.go @@ -13,6 +13,17 @@ func AssertAction(t *testing.T, actual clienttesting.Action, expected string) { } } +func AssertWarnings(t *testing.T, actual, expected []string) { + if len(actual) != len(expected) { + t.Errorf("expected %v but got: %v", expected, actual) + } + for i := 0; i < len(actual); i++ { + if actual[i] != expected[i] { + t.Errorf("expected %v but got: %v", expected, actual) + } + } +} + func AssertErrors(t *testing.T, actual, expected []error) { if len(actual) != len(expected) { t.Errorf("expected %v but got: %v", expected, actual)