From e1340df84f900f2eca81f5097bf21105412a6ab7 Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Thu, 27 Aug 2020 11:33:01 +0200 Subject: [PATCH 1/5] Verify that the api-server serves the correct CRDs that this client expects Signed-off-by: Andreas Neumann --- pkg/kudoctl/kudoinit/crd/crds.go | 76 +++++++++++++++++++++----- pkg/kudoctl/util/kudo/kudo.go | 45 ++++++++-------- pkg/kudoctl/util/kudo/kudo_test.go | 86 ++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 34 deletions(-) diff --git a/pkg/kudoctl/kudoinit/crd/crds.go b/pkg/kudoctl/kudoinit/crd/crds.go index 067a53438..e21601a0e 100644 --- a/pkg/kudoctl/kudoinit/crd/crds.go +++ b/pkg/kudoctl/kudoinit/crd/crds.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "os" + "reflect" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1" @@ -65,15 +66,31 @@ func (c Initializer) PreUpgradeVerify(client *kube.Client, result *verifier.Resu return nil } -// VerifyInstallation ensures that the CRDs are installed and have the correct and expected version +// VerifyInstallation ensures that the CRDs are installed and are the same as this CLI would install func (c Initializer) VerifyInstallation(client *kube.Client, result *verifier.Result) error { - if err := c.verifyInstallation(client.ExtClient.ApiextensionsV1beta1(), c.Operator, result); err != nil { + apiClient := client.ExtClient.ApiextensionsV1beta1() + if err := c.verifyInstallation(apiClient, c.Operator, result); err != nil { return err } - if err := c.verifyInstallation(client.ExtClient.ApiextensionsV1beta1(), c.OperatorVersion, result); err != nil { + if err := c.verifyInstallation(apiClient, c.OperatorVersion, result); err != nil { return err } - if err := c.verifyInstallation(client.ExtClient.ApiextensionsV1beta1(), c.Instance, result); err != nil { + if err := c.verifyInstallation(apiClient, c.Instance, result); err != nil { + return err + } + return nil +} + +// VerifyServedVersion ensures that the api server provides the version of the CRDs that this client understands +func (c Initializer) VerifyServedVersion(client *kube.Client, expectedVersion string, result *verifier.Result) error { + apiClient := client.ExtClient.ApiextensionsV1beta1() + if err := c.verifyServedVersion(apiClient, c.Operator.Name, expectedVersion, result); err != nil { + return err + } + if err := c.verifyServedVersion(apiClient, c.OperatorVersion.Name, expectedVersion, result); err != nil { + return err + } + if err := c.verifyServedVersion(apiClient, c.Instance.Name, expectedVersion, result); err != nil { return err } return nil @@ -105,23 +122,56 @@ func (c Initializer) verifyIsNotInstalled(client v1beta1.CustomResourceDefinitio return nil } -func (c Initializer) verifyInstallation(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition, result *verifier.Result) error { - existingCrd, err := client.CustomResourceDefinitions().Get(context.TODO(), crd.Name, v1.GetOptions{}) +func (c Initializer) getCrdForVerify(client v1beta1.CustomResourceDefinitionsGetter, crdName string, result *verifier.Result) (*apiextv1beta1.CustomResourceDefinition, error) { + existingCrd, err := client.CustomResourceDefinitions().Get(context.TODO(), crdName, v1.GetOptions{}) if err != nil { if os.IsTimeout(err) { - return err + return nil, err } if kerrors.IsNotFound(err) { - result.AddErrors(fmt.Sprintf("CRD %s is not installed", crd.Name)) - return nil + result.AddErrors(fmt.Sprintf("CRD %s is not installed", crdName)) + return nil, nil + } + return nil, fmt.Errorf("failed to retrieve CRD %s: %v", crdName, err) + } + return existingCrd, nil +} + +func (c Initializer) verifyInstallation(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition, result *verifier.Result) error { + existingCrd, err := c.getCrdForVerify(client, crd.Name, result) + if err != nil || existingCrd == nil { + return err + } + if !reflect.DeepEqual(existingCrd.Spec.Versions, crd.Spec.Versions) { + result.AddErrors(fmt.Sprintf("Installed CRD versions do not match expected CRD versions (%v vs %v).", existingCrd.Spec.Versions, crd.Spec.Versions)) + } + clog.V(2).Printf("CRD %s is installed with versions %v", crd.Name, existingCrd.Spec.Versions) + return nil +} + +func (c Initializer) verifyServedVersion(client v1beta1.CustomResourceDefinitionsGetter, crdName, version string, result *verifier.Result) error { + existingCrd, err := c.getCrdForVerify(client, crdName, result) + if err != nil || existingCrd == nil { + return err + } + + var expectedVersion *apiextv1beta1.CustomResourceDefinitionVersion + var allNames = []string{} + for _, v := range existingCrd.Spec.Versions { + v := v + allNames = append(allNames, v.Name) + if v.Name == version { + expectedVersion = &v + break } - return fmt.Errorf("failed to retrieve CRD %s: %v", crd.Name, err) } - if existingCrd.Spec.Version != crd.Spec.Version { - result.AddErrors(fmt.Sprintf("Installed CRD %s has invalid version %s, expected %s", crd.Name, existingCrd.Spec.Version, crd.Spec.Version)) + if expectedVersion == nil { + result.AddErrors(fmt.Sprintf("Expected API version %s was not found, api-server only supports %v. Please update your KUDO CLI.", version, allNames)) return nil } - clog.V(2).Printf("CRD %s is installed with version %s", crd.Name, existingCrd.Spec.Versions[0].Name) + if !expectedVersion.Served { + result.AddErrors(fmt.Sprintf("Expected API version %s is known to api-server, but is not served. Please update your KUDO CLI.", version)) + } return nil } diff --git a/pkg/kudoctl/util/kudo/kudo.go b/pkg/kudoctl/util/kudo/kudo.go index df5a44fb3..5edf4816b 100644 --- a/pkg/kudoctl/util/kudo/kudo.go +++ b/pkg/kudoctl/util/kudo/kudo.go @@ -58,25 +58,11 @@ func NewClient(kubeConfigPath string, requestTimeout int64, validateInstall bool // NewClient creates new KUDO Client func NewClientForConfig(config *rest.Config, validateInstall bool) (*Client, error) { - kubeClient, err := kube.GetKubeClientForConfig(config) if err != nil { return nil, clog.Errorf("could not get Kubernetes client: %s", err) } - result := verifier.NewResult() - err = crd.NewInitializer().VerifyInstallation(kubeClient, &result) - if err != nil { - return nil, fmt.Errorf("failed to run crd verification: %v", err) - } - if !result.IsValid() { - clog.V(0).Printf("KUDO CRDs are not set up correctly. Do you need to run kudo init?") - - if validateInstall { - return nil, fmt.Errorf("CRDs invalid: %v", result.ErrorsAsString()) - } - } - // create the kudo clientset kudoClientset, err := versioned.NewForConfig(config) if err != nil { @@ -84,14 +70,17 @@ func NewClientForConfig(config *rest.Config, validateInstall bool) (*Client, err } // create the kubernetes clientset - kubeClientset, err := kubernetes.NewForConfig(config) - if err != nil { - return nil, err - } - return &Client{ + client := &Client{ kudoClientset: kudoClientset, - KubeClientset: kubeClientset, - }, nil + KubeClientset: kubeClient.KubeClient, + } + + validationErr := client.VerifyServedCRDs(kubeClient) + if validateInstall && validationErr != nil { + return nil, validationErr + } + + return client, nil } // NewClientFromK8s creates KUDO client from kubernetes client interface @@ -102,6 +91,20 @@ func NewClientFromK8s(kudo versioned.Interface, kube kubernetes.Interface) *Clie return &result } +func (c *Client) VerifyServedCRDs(kubeClient *kube.Client) error { + result := verifier.NewResult() + err := crd.NewInitializer().VerifyServedVersion(kubeClient, v1beta1.SchemeGroupVersion.Version, &result) + if err != nil { + return fmt.Errorf("failed to run crd verification: %v", err) + } + if !result.IsValid() { + clog.V(0).Printf("KUDO CRDs are not served in the expected version.") + return fmt.Errorf("CRDs invalid: %v", result.ErrorsAsString()) + } + + return nil +} + // OperatorExistsInCluster checks if a given Operator object is installed on the current k8s cluster func (c *Client) OperatorExistsInCluster(name, namespace string) bool { operator, err := c.kudoClientset.KudoV1beta1().Operators(namespace).Get(context.TODO(), name, v1.GetOptions{}) diff --git a/pkg/kudoctl/util/kudo/kudo_test.go b/pkg/kudoctl/util/kudo/kudo_test.go index 918d5c315..c49dbc9b0 100644 --- a/pkg/kudoctl/util/kudo/kudo_test.go +++ b/pkg/kudoctl/util/kudo/kudo_test.go @@ -8,11 +8,16 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" + apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + apiextensionfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + dynamicfake "k8s.io/client-go/dynamic/fake" kubefake "k8s.io/client-go/kubernetes/fake" "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" "github.com/kudobuilder/kudo/pkg/client/clientset/versioned/fake" + "github.com/kudobuilder/kudo/pkg/kudoctl/kube" "github.com/kudobuilder/kudo/pkg/util/convert" "github.com/kudobuilder/kudo/pkg/util/kudo" ) @@ -23,6 +28,87 @@ func newTestSimpleK2o() *Client { return NewClientFromK8s(fake.NewSimpleClientset(), kubefake.NewSimpleClientset()) } +func newFakeKubeClient() *kube.Client { + client := kubefake.NewSimpleClientset() + extClient := apiextensionfake.NewSimpleClientset() + dynamicClient := dynamicfake.NewSimpleDynamicClient(runtime.NewScheme()) + + return &kube.Client{ + KubeClient: client, + ExtClient: extClient, + DynamicClient: dynamicClient} +} + +func TestKudoClient_ValidateServedCrds(t *testing.T) { + crdWrongVersion := apiextv1beta1.CustomResourceDefinition{ + Spec: apiextv1beta1.CustomResourceDefinitionSpec{ + Versions: []apiextv1beta1.CustomResourceDefinitionVersion{ + { + Name: "v1beta2", + Served: true, + }, + }, + }, + } + crdNotServed := apiextv1beta1.CustomResourceDefinition{ + Spec: apiextv1beta1.CustomResourceDefinitionSpec{ + Versions: []apiextv1beta1.CustomResourceDefinitionVersion{ + { + Name: "v1beta1", + Served: false, + }, + { + Name: "v1beta2", + Served: true, + }, + }, + }, + } + + tests := []struct { + name string + crdBase *apiextv1beta1.CustomResourceDefinition + err string + }{ + {name: "no crd", err: `CRDs invalid: CRD operators.kudo.dev is not installed +CRD operatorversions.kudo.dev is not installed +CRD instances.kudo.dev is not installed +`}, + {name: "wrong version", crdBase: &crdWrongVersion, err: `CRDs invalid: Expected API version v1beta1 was not found, api-server only supports [v1beta2]. Please update your KUDO CLI. +Expected API version v1beta1 was not found, api-server only supports [v1beta2]. Please update your KUDO CLI. +Expected API version v1beta1 was not found, api-server only supports [v1beta2]. Please update your KUDO CLI. +`}, + {name: "not served", crdBase: &crdNotServed, err: `CRDs invalid: Expected API version v1beta1 is known to api-server, but is not served. Please update your KUDO CLI. +Expected API version v1beta1 is known to api-server, but is not served. Please update your KUDO CLI. +Expected API version v1beta1 is known to api-server, but is not served. Please update your KUDO CLI. +`}, + } + + crdNames := []string{"instances.kudo.dev", "operators.kudo.dev", "operatorversions.kudo.dev"} + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + kudoClient := newTestSimpleK2o() + kubeClient := newFakeKubeClient() + + if tt.crdBase != nil { + for _, crdName := range crdNames { + crd := tt.crdBase.DeepCopy() + crd.Name = crdName + _, _ = kubeClient.ExtClient.ApiextensionsV1beta1().CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{}) + } + } + + err := kudoClient.VerifyServedCRDs(kubeClient) + + assert.EqualError(t, err, tt.err) + + }) + } + +} + func TestKudoClient_OperatorExistsInCluster(t *testing.T) { obj := v1beta1.Operator{ From 8e353cdb18345a3b60b4530330d98ffb9e036732 Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Wed, 2 Sep 2020 17:02:30 +0200 Subject: [PATCH 2/5] Fixed integration tests and added additional test case Signed-off-by: Andreas Neumann --- pkg/kudoctl/kudoinit/crd/crds.go | 2 +- pkg/kudoctl/util/kudo/kudo_test.go | 42 ++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/pkg/kudoctl/kudoinit/crd/crds.go b/pkg/kudoctl/kudoinit/crd/crds.go index a1904b105..f2a9ba90b 100644 --- a/pkg/kudoctl/kudoinit/crd/crds.go +++ b/pkg/kudoctl/kudoinit/crd/crds.go @@ -160,7 +160,7 @@ func (c Initializer) verifyServedVersion(client v1beta1.CustomResourceDefinition return err } if err := health.IsHealthy(existingCrd); err != nil { - result.AddErrors(fmt.Sprintf("Installed CRD %s is not healthy: %v", crdName, err)) + result.AddErrors(err.Error()) return nil } diff --git a/pkg/kudoctl/util/kudo/kudo_test.go b/pkg/kudoctl/util/kudo/kudo_test.go index c49dbc9b0..f0cbb1ed5 100644 --- a/pkg/kudoctl/util/kudo/kudo_test.go +++ b/pkg/kudoctl/util/kudo/kudo_test.go @@ -49,6 +49,14 @@ func TestKudoClient_ValidateServedCrds(t *testing.T) { }, }, }, + Status: apiextv1beta1.CustomResourceDefinitionStatus{ + Conditions: []apiextv1beta1.CustomResourceDefinitionCondition{ + { + Type: apiextv1beta1.Established, + Status: apiextv1beta1.ConditionTrue, + }, + }, + }, } crdNotServed := apiextv1beta1.CustomResourceDefinition{ Spec: apiextv1beta1.CustomResourceDefinitionSpec{ @@ -63,6 +71,36 @@ func TestKudoClient_ValidateServedCrds(t *testing.T) { }, }, }, + Status: apiextv1beta1.CustomResourceDefinitionStatus{ + Conditions: []apiextv1beta1.CustomResourceDefinitionCondition{ + { + Type: apiextv1beta1.Established, + Status: apiextv1beta1.ConditionTrue, + }, + }, + }, + } + crdUnHealthy := apiextv1beta1.CustomResourceDefinition{ + Spec: apiextv1beta1.CustomResourceDefinitionSpec{ + Versions: []apiextv1beta1.CustomResourceDefinitionVersion{ + { + Name: "v1beta2", + Served: true, + }, + }, + }, + Status: apiextv1beta1.CustomResourceDefinitionStatus{ + Conditions: []apiextv1beta1.CustomResourceDefinitionCondition{ + { + Type: apiextv1beta1.Established, + Status: apiextv1beta1.ConditionFalse, + }, + { + Type: apiextv1beta1.Terminating, + Status: apiextv1beta1.ConditionTrue, + }, + }, + }, } tests := []struct { @@ -81,6 +119,10 @@ Expected API version v1beta1 was not found, api-server only supports [v1beta2]. {name: "not served", crdBase: &crdNotServed, err: `CRDs invalid: Expected API version v1beta1 is known to api-server, but is not served. Please update your KUDO CLI. Expected API version v1beta1 is known to api-server, but is not served. Please update your KUDO CLI. Expected API version v1beta1 is known to api-server, but is not served. Please update your KUDO CLI. +`}, + {name: "unhealthy", crdBase: &crdUnHealthy, err: `CRDs invalid: CRD operators.kudo.dev is not healthy ( Conditions: [{Established False 0001-01-01 00:00:00 +0000 UTC } {Terminating True 0001-01-01 00:00:00 +0000 UTC }] ) +CRD operatorversions.kudo.dev is not healthy ( Conditions: [{Established False 0001-01-01 00:00:00 +0000 UTC } {Terminating True 0001-01-01 00:00:00 +0000 UTC }] ) +CRD instances.kudo.dev is not healthy ( Conditions: [{Established False 0001-01-01 00:00:00 +0000 UTC } {Terminating True 0001-01-01 00:00:00 +0000 UTC }] ) `}, } From ff6db55a0f0dbf97ce717cccd598ab592d48afe9 Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Wed, 2 Sep 2020 17:52:14 +0200 Subject: [PATCH 3/5] Undo things that were merged incorrectly Signed-off-by: Andreas Neumann --- pkg/engine/task/task_apply.go | 4 ++-- pkg/kudoctl/cmd/init.go | 11 +++++----- pkg/kudoctl/kudoinit/prereq/namespace.go | 6 ++++- pkg/kudoctl/kudoinit/setup/setup.go | 20 +++++++++++++++++ pkg/kudoctl/kudoinit/setup/wait.go | 28 ++++++------------------ 5 files changed, 39 insertions(+), 30 deletions(-) diff --git a/pkg/engine/task/task_apply.go b/pkg/engine/task/task_apply.go index 0f522a108..510e255a4 100644 --- a/pkg/engine/task/task_apply.go +++ b/pkg/engine/task/task_apply.go @@ -118,7 +118,7 @@ func applyResources(rr []runtime.Object, ctx Context) ([]runtime.Object, error) switch { case apierrors.IsNotFound(err): // create resource if it doesn't exist if err := addLastAppliedConfigAnnotation(r); err != nil { - return nil, fmt.Errorf("failed to add last applied config annotation to %s/%s: %w", key.Namespace, key.Name, err) + return nil, fmt.Errorf("failed to add last applied config annotation to a %s %s: %w", r.GetObjectKind().GroupVersionKind(), key, err) } err = ctx.Client.Create(context.TODO(), r) @@ -137,7 +137,7 @@ func applyResources(rr []runtime.Object, ctx Context) ([]runtime.Object, error) default: // update existing resource err := patchResource(r, existing, ctx) if err != nil { - return nil, fmt.Errorf("failed to patch %s/%s: %w", key.Namespace, key.Name, err) + return nil, fmt.Errorf("failed to patch a %s %s: %w", r.GetObjectKind().GroupVersionKind(), key, err) } applied = append(applied, r) } diff --git a/pkg/kudoctl/cmd/init.go b/pkg/kudoctl/cmd/init.go index 27b1edaf6..850e563b9 100644 --- a/pkg/kudoctl/cmd/init.go +++ b/pkg/kudoctl/cmd/init.go @@ -218,7 +218,7 @@ func (initCmd *initCmd) run() error { if initCmd.wait { clog.Printf("⌛Waiting for KUDO controller to be ready in your cluster...") - err := setup.WatchKUDOUntilReady(initCmd.client.KubeClient, opts, initCmd.timeout) + err := setup.WatchKUDOUntilReady(installer, initCmd.client, initCmd.timeout) if err != nil { return errors.New("watch timed out, readiness uncertain") } @@ -287,13 +287,12 @@ func (initCmd *initCmd) runYamlOutput(installer kudoinit.Artifacter) error { // verifyExistingInstallation checks if the current installation is valid and as expected func (initCmd *initCmd) verifyExistingInstallation(v kudoinit.InstallVerifier) error { clog.V(4).Printf("verify existing installation") - result := verifier.NewResult() - if err := v.VerifyInstallation(initCmd.client, &result); err != nil { + ok, err := setup.VerifyExistingInstallation(v, initCmd.client, initCmd.out) + if err != nil { return err } - result.PrintWarnings(initCmd.out) - if !result.IsValid() { - result.PrintErrors(initCmd.out) + if !ok { + return fmt.Errorf("KUDO installation is not valid") } return nil } diff --git a/pkg/kudoctl/kudoinit/prereq/namespace.go b/pkg/kudoctl/kudoinit/prereq/namespace.go index 959cf3974..ab6cd9314 100644 --- a/pkg/kudoctl/kudoinit/prereq/namespace.go +++ b/pkg/kudoctl/kudoinit/prereq/namespace.go @@ -9,6 +9,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "github.com/kudobuilder/kudo/pkg/engine/health" "github.com/kudobuilder/kudo/pkg/kudoctl/clog" "github.com/kudobuilder/kudo/pkg/kudoctl/kube" "github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit" @@ -64,7 +65,7 @@ func (o KudoNamespace) PreUpgradeVerify(client *kube.Client, result *verifier.Re } func (o KudoNamespace) VerifyInstallation(client *kube.Client, result *verifier.Result) error { - _, err := client.KubeClient.CoreV1().Namespaces().Get(context.TODO(), o.opts.Namespace, metav1.GetOptions{}) + ns, err := client.KubeClient.CoreV1().Namespaces().Get(context.TODO(), o.opts.Namespace, metav1.GetOptions{}) if err != nil { if kerrors.IsNotFound(err) { result.AddErrors(fmt.Sprintf("namespace %s does not exist", o.opts.Namespace)) @@ -72,6 +73,9 @@ func (o KudoNamespace) VerifyInstallation(client *kube.Client, result *verifier. } return err } + if err := health.IsHealthy(ns); err != nil { + result.AddErrors(fmt.Sprintf("namespace %s is not healthy: %v", o.opts.Namespace, err)) + } return nil } diff --git a/pkg/kudoctl/kudoinit/setup/setup.go b/pkg/kudoctl/kudoinit/setup/setup.go index 5e99c0295..b1073841b 100644 --- a/pkg/kudoctl/kudoinit/setup/setup.go +++ b/pkg/kudoctl/kudoinit/setup/setup.go @@ -2,6 +2,7 @@ package setup import ( "fmt" + "io" "k8s.io/apimachinery/pkg/runtime" @@ -167,3 +168,22 @@ func (i *Installer) Resources() []runtime.Object { return allManifests } + +// verifyExistingInstallation checks if the current installation is valid and as expected +func VerifyExistingInstallation(v kudoinit.InstallVerifier, client *kube.Client, out io.Writer) (bool, error) { + clog.V(4).Printf("verify existing installation") + result := verifier.NewResult() + if err := v.VerifyInstallation(client, &result); err != nil { + return false, err + } + if out != nil { + result.PrintWarnings(out) + } + if !result.IsValid() { + if out != nil { + result.PrintErrors(out) + } + return false, nil + } + return true, nil +} diff --git a/pkg/kudoctl/kudoinit/setup/wait.go b/pkg/kudoctl/kudoinit/setup/wait.go index 1eb9c163d..4d4c110d8 100644 --- a/pkg/kudoctl/kudoinit/setup/wait.go +++ b/pkg/kudoctl/kudoinit/setup/wait.go @@ -1,36 +1,22 @@ package setup import ( - "context" "time" - "github.com/kudobuilder/kudo/pkg/engine/health" + "github.com/kudobuilder/kudo/pkg/kudoctl/kube" "k8s.io/apimachinery/pkg/util/wait" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" - appsv1 "k8s.io/client-go/kubernetes/typed/apps/v1" - "github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit" ) -// WatchKUDOUntilReady waits for the KUDO pod to become available. +// WatchKUDOUntilReady waits for the KUDO installation to become available. // // Returns no error if it exists. If the timeout was reached and it could not find the pod, it returns error. -func WatchKUDOUntilReady(client kubernetes.Interface, opts kudoinit.Options, timeout int64) error { +func WatchKUDOUntilReady(v kudoinit.InstallVerifier, client *kube.Client, timeout int64) error { return wait.PollImmediate(500*time.Millisecond, time.Duration(timeout)*time.Second, - func() (bool, error) { return verifyKudoStatefulset(client.AppsV1(), opts.Namespace) }) -} - -func verifyKudoStatefulset(client appsv1.StatefulSetsGetter, namespace string) (bool, error) { - ss, err := client.StatefulSets(namespace).Get(context.TODO(), kudoinit.DefaultManagerName, metav1.GetOptions{}) - if err != nil || ss == nil { - return false, err - } - err = health.IsHealthy(ss) - if err != nil { - return false, nil - } - return true, nil + func() (bool, error) { + return VerifyExistingInstallation(v, client, nil) + }, + ) } From abaa4a66f8a79ccc450d36012a3a843fe179eda2 Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Wed, 2 Sep 2020 17:54:03 +0200 Subject: [PATCH 4/5] Undo things that were merged incorrectly Signed-off-by: Andreas Neumann --- pkg/engine/health/health.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/engine/health/health.go b/pkg/engine/health/health.go index 936e7595d..6a324e862 100644 --- a/pkg/engine/health/health.go +++ b/pkg/engine/health/health.go @@ -121,6 +121,12 @@ func IsHealthy(obj runtime.Object) error { } return fmt.Errorf("pod %q is not running yet: %s", obj.Name, obj.Status.Phase) + case *corev1.Namespace: + if obj.Status.Phase == corev1.NamespaceActive { + return nil + } + return fmt.Errorf("namespace %s is not active: %s", obj.Name, obj.Status.Phase) + // unless we build logic for what a healthy object is, assume it's healthy when created. default: log.Printf("HealthUtil: Unknown type %s is marked healthy by default", reflect.TypeOf(obj)) From f937c1175a7fb56c5d622f85c879fe50fd666875 Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Mon, 14 Sep 2020 12:10:54 +0200 Subject: [PATCH 5/5] Code review changes Signed-off-by: Andreas Neumann --- pkg/kudoctl/kudoinit/crd/crds.go | 11 +++++++---- pkg/kudoctl/util/kudo/kudo_test.go | 12 ++++++------ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/pkg/kudoctl/kudoinit/crd/crds.go b/pkg/kudoctl/kudoinit/crd/crds.go index f2a9ba90b..0cbf98a55 100644 --- a/pkg/kudoctl/kudoinit/crd/crds.go +++ b/pkg/kudoctl/kudoinit/crd/crds.go @@ -67,7 +67,7 @@ func (c Initializer) PreUpgradeVerify(client *kube.Client, result *verifier.Resu return nil } -// VerifyInstallation ensures that the CRDs are installed and are the same as this CLI would install +// VerifyInstallation ensures that all CRDs are installed and are the same as this CLI would install func (c Initializer) VerifyInstallation(client *kube.Client, result *verifier.Result) error { apiClient := client.ExtClient.ApiextensionsV1beta1() if err := c.verifyInstallation(apiClient, c.Operator, result); err != nil { @@ -82,7 +82,7 @@ func (c Initializer) VerifyInstallation(client *kube.Client, result *verifier.Re return nil } -// VerifyServedVersion ensures that the api server provides the version of the CRDs that this client understands +// VerifyServedVersion ensures that the api server provides the correct version of all CRDs that this client understands func (c Initializer) VerifyServedVersion(client *kube.Client, expectedVersion string, result *verifier.Result) error { apiClient := client.ExtClient.ApiextensionsV1beta1() if err := c.verifyServedVersion(apiClient, c.Operator.Name, expectedVersion, result); err != nil { @@ -111,6 +111,7 @@ func (c Initializer) Install(client *kube.Client) error { return nil } +// verifyIsNotInstalled is used to ensure that the cluster has no old KUDO version installed func (c Initializer) verifyIsNotInstalled(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition, result *verifier.Result) error { _, err := client.CustomResourceDefinitions().Get(context.TODO(), crd.Name, v1.GetOptions{}) if err != nil { @@ -138,6 +139,7 @@ func (c Initializer) getCrdForVerify(client v1beta1.CustomResourceDefinitionsGet return existingCrd, nil } +// VerifyInstallation ensures that a single CRD is installed and is the same as this CLI would install func (c Initializer) verifyInstallation(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition, result *verifier.Result) error { existingCrd, err := c.getCrdForVerify(client, crd.Name, result) if err != nil || existingCrd == nil { @@ -154,6 +156,7 @@ func (c Initializer) verifyInstallation(client v1beta1.CustomResourceDefinitions return nil } +// VerifyServedVersion ensures that the api server provides the correct version of a specific CRDs that this client understands func (c Initializer) verifyServedVersion(client v1beta1.CustomResourceDefinitionsGetter, crdName, version string, result *verifier.Result) error { existingCrd, err := c.getCrdForVerify(client, crdName, result) if err != nil || existingCrd == nil { @@ -175,11 +178,11 @@ func (c Initializer) verifyServedVersion(client v1beta1.CustomResourceDefinition } } if expectedVersion == nil { - result.AddErrors(fmt.Sprintf("Expected API version %s was not found, api-server only supports %v. Please update your KUDO CLI.", version, allNames)) + result.AddErrors(fmt.Sprintf("Expected API version %s was not found for %s, api-server only supports %v. Please update your KUDO CLI.", version, crdName, allNames)) return nil } if !expectedVersion.Served { - result.AddErrors(fmt.Sprintf("Expected API version %s is known to api-server, but is not served. Please update your KUDO CLI.", version)) + result.AddErrors(fmt.Sprintf("Expected API version %s for %s is known to api-server, but is not served. Please update your KUDO CLI.", version, crdName)) } return nil } diff --git a/pkg/kudoctl/util/kudo/kudo_test.go b/pkg/kudoctl/util/kudo/kudo_test.go index 17edc5a12..0cf892182 100644 --- a/pkg/kudoctl/util/kudo/kudo_test.go +++ b/pkg/kudoctl/util/kudo/kudo_test.go @@ -113,13 +113,13 @@ func TestKudoClient_ValidateServedCrds(t *testing.T) { CRD operatorversions.kudo.dev is not installed CRD instances.kudo.dev is not installed `}, - {name: "wrong version", crdBase: &crdWrongVersion, err: `CRDs invalid: Expected API version v1beta1 was not found, api-server only supports [v1beta2]. Please update your KUDO CLI. -Expected API version v1beta1 was not found, api-server only supports [v1beta2]. Please update your KUDO CLI. -Expected API version v1beta1 was not found, api-server only supports [v1beta2]. Please update your KUDO CLI. + {name: "wrong version", crdBase: &crdWrongVersion, err: `CRDs invalid: Expected API version v1beta1 was not found for operators.kudo.dev, api-server only supports [v1beta2]. Please update your KUDO CLI. +Expected API version v1beta1 was not found for operatorversions.kudo.dev, api-server only supports [v1beta2]. Please update your KUDO CLI. +Expected API version v1beta1 was not found for instances.kudo.dev, api-server only supports [v1beta2]. Please update your KUDO CLI. `}, - {name: "not served", crdBase: &crdNotServed, err: `CRDs invalid: Expected API version v1beta1 is known to api-server, but is not served. Please update your KUDO CLI. -Expected API version v1beta1 is known to api-server, but is not served. Please update your KUDO CLI. -Expected API version v1beta1 is known to api-server, but is not served. Please update your KUDO CLI. + {name: "not served", crdBase: &crdNotServed, err: `CRDs invalid: Expected API version v1beta1 for operators.kudo.dev is known to api-server, but is not served. Please update your KUDO CLI. +Expected API version v1beta1 for operatorversions.kudo.dev is known to api-server, but is not served. Please update your KUDO CLI. +Expected API version v1beta1 for instances.kudo.dev is known to api-server, but is not served. Please update your KUDO CLI. `}, {name: "unhealthy", crdBase: &crdUnHealthy, err: `CRDs invalid: CRD operators.kudo.dev is not healthy ( Conditions: [{Established False 0001-01-01 00:00:00 +0000 UTC } {Terminating True 0001-01-01 00:00:00 +0000 UTC }] ) CRD operatorversions.kudo.dev is not healthy ( Conditions: [{Established False 0001-01-01 00:00:00 +0000 UTC } {Terminating True 0001-01-01 00:00:00 +0000 UTC }] )