Wait until the manager is actually gone before continuing the upgrade (…

…#1635)

* Wait until the manager is actually gone before continuing the upgrade
* Use generic DeleteAndWait functions, cleanup

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>

pkg/kubernetes/client.go

@@ -1,10 +1,19 @@
 package kubernetes
 
 import (
+	"context"
 	"fmt"
+	"time"
 
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/discovery"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+
+	"github.com/kudobuilder/kudo/pkg/kudoctl/clog"
 )
 
 func GetDiscoveryClient(mgr manager.Manager) (*discovery.DiscoveryClient, error) {
@@ -15,3 +24,47 @@ func GetDiscoveryClient(mgr manager.Manager) (*discovery.DiscoveryClient, error)
 	}
 	return dc, nil
 }
+
+// DeleteAndWait deletes the given runtime object and waits until it is fully deleted
+func DeleteAndWait(c client.Client, obj runtime.Object, options ...client.DeleteOption) error {
+	err := c.Delete(context.TODO(), obj, options...)
+
+	if err != nil {
+		key := ObjectKey(obj)
+
+		if kerrors.IsNotFound(err) {
+			// Obj is already deleted, we can return directly
+			clog.V(4).Printf("Deleting obj %s/%s is already NotFound, return now", key.Namespace, key.Name)
+			return nil
+		}
+		return fmt.Errorf("failed to delete %s/%s: %v", key.Namespace, key.Name, err)
+	}
+
+	return WaitForDelete(c, obj)
+}
+
+// WaitForDelete waits for the provided runtime object to be deleted from cluster
+func WaitForDelete(c client.Client, obj runtime.Object) error {
+	key := ObjectKey(obj)
+	clog.V(4).Printf("Waiting for obj %s/%s to be finally deleted", key.Namespace, key.Name)
+
+	// Wait for resources to be deleted.
+	return wait.PollImmediate(250*time.Millisecond, 30*time.Second, func() (done bool, err error) {
+		err = c.Get(context.TODO(), key, obj.DeepCopyObject())
+		clog.V(6).Printf("Fetched %s/%s to wait for delete: %v", key.Namespace, key.Name, err)
+
+		if err != nil && kerrors.IsNotFound(err) {
+			return true, nil
+		}
+		return false, err
+	})
+}
+
+// ObjectKey returns an instantiated ObjectKey for the provided object.
+func ObjectKey(obj runtime.Object) client.ObjectKey {
+	m, _ := meta.Accessor(obj)
+	return client.ObjectKey{
+		Name:      m.GetName(),
+		Namespace: m.GetNamespace(),
+	}
+}

pkg/kudoctl/cmd/init_integration_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit/crd"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit/prereq"
+	"github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo"
 )
 
 var testenv testutils.TestEnvironment
@@ -58,6 +59,20 @@ const (
 	manifestsDir            = "../../../config/crds/"
 )
 
+func TestKudoClientValidate(t *testing.T) {
+	tests := []struct {
+		err string
+	}{
+		{"CRDs invalid: CRD operators.kudo.dev is not installed"}, // verify that NewClient tries to validate CRDs
+	}
+
+	for _, tt := range tests {
+		_, err := kudo.NewClientForConfig(testenv.Config, true)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), tt.err)
+	}
+}
+
 func TestCrds_Config(t *testing.T) {
 	crds := crd.NewInitializer()
 	assertManifestFileMatch(t, operatorFileName, crds.Operator)
@@ -427,5 +442,7 @@ func getKubeClient(t *testing.T) *kube.Client {
 	assert.NoError(t, err)
 	xc, err := apiextensionsclient.NewForConfig(testenv.Config)
 	assert.NoError(t, err)
-	return &kube.Client{KubeClient: c, ExtClient: xc}
+	cc, err := client.New(testenv.Config, client.Options{})
+	assert.NoError(t, err)
+	return &kube.Client{KubeClient: c, ExtClient: xc, CtrlClient: cc}
 }

pkg/kudoctl/cmd/init_test.go

@@ -21,7 +21,9 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	yamlutil "k8s.io/apimachinery/pkg/util/yaml"
 	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/kubernetes/scheme"
 	testing2 "k8s.io/client-go/testing"
+	fake2 "sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	"github.com/kudobuilder/kudo/pkg/kudoctl/clog"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/kube"
@@ -31,21 +33,42 @@ import (
 
 var updateGolden = flag.Bool("update", false, "update .golden files and manifests in /config/crd")
 
-func TestInitCmd_exists(t *testing.T) {
+type fakeClient struct {
+	client *kube.Client
+	fc     *fake.Clientset
+	fc2    *apiextfake.Clientset
+}
 
-	var buf bytes.Buffer
-	fc := fake.NewSimpleClientset(&v1beta1.Deployment{
+func newFakeClient(objs ...runtime.Object) fakeClient {
+	fc := fake.NewSimpleClientset(objs...)
+	fc2 := apiextfake.NewSimpleClientset()
+	cc := fake2.NewFakeClientWithScheme(scheme.Scheme, objs...)
+
+	return fakeClient{
+		client: &kube.Client{
+			KubeClient: fc,
+			ExtClient:  fc2,
+			CtrlClient: cc,
+		},
+		fc:  fc,
+		fc2: fc2,
+	}
+}
+
+func TestInitCmd_exists(t *testing.T) {
+	c := newFakeClient(&v1beta1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: "kudo-system",
 			Name:      "kudo-manager-deploy",
 		},
 	})
-	fc2 := apiextfake.NewSimpleClientset()
+
+	var buf bytes.Buffer
 
 	cmd := &initCmd{
 		out:                 &buf,
 		fs:                  afero.NewMemMapFs(),
-		client:              &kube.Client{KubeClient: fc, ExtClient: fc2},
+		client:              c.client,
 		image:               "fake",
 		selfSignedWebhookCA: true,
 	}
@@ -64,14 +87,10 @@ func TestInitCmd_exists(t *testing.T) {
 
 // TestInitCmd_output tests that init -o can be decoded
 func TestInitCmd_output(t *testing.T) {
-	fc := fake.NewSimpleClientset()
-	client := &kube.Client{
-		KubeClient: fc,
-		ExtClient:  apiextfake.NewSimpleClientset(),
-	}
+	c := newFakeClient()
 
-	MockCRD(client, "certificates.cert-manager.io", "v1alpha2")
-	MockCRD(client, "issuers.cert-manager.io", "v1alpha2")
+	MockCRD(c.client, "certificates.cert-manager.io", "v1alpha2")
+	MockCRD(c.client, "issuers.cert-manager.io", "v1alpha2")
 
 	tests := []string{"yaml"}
 	for _, s := range tests {
@@ -80,7 +99,7 @@ func TestInitCmd_output(t *testing.T) {
 		cmd := &initCmd{
 			out:     &buf,
 			errOut:  &errOut,
-			client:  client,
+			client:  c.client,
 			output:  s,
 			dryRun:  true,
 			version: "dev",
@@ -91,7 +110,7 @@ func TestInitCmd_output(t *testing.T) {
 		}
 		// ensure no modifying calls against the server
 		forbiddenVerbs := []string{"create", "update", "patch", "delete"}
-		for _, a := range fc.Actions() {
+		for _, a := range c.fc.Actions() {
 			if funk.Contains(forbiddenVerbs, a.GetVerb()) {
 				t.Errorf("got modifying server call: %v", a)
 			}
@@ -142,15 +161,10 @@ func TestInitCmd_yamlOutput(t *testing.T) {
 			Name:     "cluster-admin",
 		},
 	}
+	c := newFakeClient(crb, customNs, customSa)
 
-	fc := fake.NewSimpleClientset(crb, customNs, customSa)
-	client := &kube.Client{
-		KubeClient: fc,
-		ExtClient:  apiextfake.NewSimpleClientset(),
-	}
-
-	MockCRD(client, "certificates.cert-manager.io", "v1alpha2")
-	MockCRD(client, "issuers.cert-manager.io", "v1alpha2")
+	MockCRD(c.client, "certificates.cert-manager.io", "v1alpha2")
+	MockCRD(c.client, "issuers.cert-manager.io", "v1alpha2")
 
 	tests := []struct {
 		name       string
@@ -167,7 +181,7 @@ func TestInitCmd_yamlOutput(t *testing.T) {
 		fs := afero.NewMemMapFs()
 		out := &bytes.Buffer{}
 		errOut := &bytes.Buffer{}
-		initCmd := newInitCmd(fs, out, errOut, client)
+		initCmd := newInitCmd(fs, out, errOut, c.client)
 
 		Settings.AddFlags(initCmd.Flags())
 

pkg/kudoctl/kube/config.go

@@ -8,6 +8,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kudobuilder/kudo/pkg/kudoctl/clog"
 )
@@ -17,6 +18,7 @@ type Client struct {
 	KubeClient    kubernetes.Interface
 	ExtClient     apiextensionsclient.Interface
 	DynamicClient dynamic.Interface
+	CtrlClient    client.Client
 }
 
 // GetConfig returns a Kubernetes client config for a given kubeconfig.
@@ -48,7 +50,11 @@ func GetKubeClient(kubeconfig string) (*Client, error) {
 	if err != nil {
 		return nil, err
 	}
-	client, err := kubernetes.NewForConfig(config)
+	return GetKubeClientForConfig(config)
+}
+
+func GetKubeClientForConfig(config *rest.Config) (*Client, error) {
+	kubeClient, err := kubernetes.NewForConfig(config)
 	if err != nil {
 		return nil, fmt.Errorf("could not get Kubernetes client: %s", err)
 	}
@@ -60,9 +66,15 @@ func GetKubeClient(kubeconfig string) (*Client, error) {
 	if err != nil {
 		return nil, fmt.Errorf("could not create Kubernetes dynamic client: %s", err)
 	}
+	ctrlClient, err := client.New(config, client.Options{})
+	if err != nil {
+		return nil, fmt.Errorf("could not create Controller Runtime client: %s", err)
+	}
 
 	return &Client{
-		KubeClient:    client,
+		KubeClient:    kubeClient,
 		ExtClient:     extClient,
-		DynamicClient: dynamicClient}, nil
+		DynamicClient: dynamicClient,
+		CtrlClient:    ctrlClient,
+	}, nil
 }