Merge pull request #4265 from fabriziopandini/clustectl-v1alpha3-shou…

…ld-block-on-v1alpha4-clusters

⚠️ Clustectl v1alpha3 should block on v1alpha4 clusters

cmd/clusterctl/client/cluster/inventory.go

@@ -17,14 +17,15 @@ limitations under the License.
 package cluster
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/pkg/errors"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/sets"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
 	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/config"
 	logf "sigs.k8s.io/cluster-api/cmd/clusterctl/log"
@@ -39,6 +40,41 @@ const (
 	waitInventoryCRDTimeout  = 1 * time.Minute
 )
 
+// CheckCAPIContractOption is some configuration that modifies options for CheckCAPIContract.
+type CheckCAPIContractOption interface {
+	// Apply applies this configuration to the given CheckCAPIContractOptions.
+	Apply(*CheckCAPIContractOptions)
+}
+
+// CheckCAPIContractOptions contains options for CheckCAPIContract.
+type CheckCAPIContractOptions struct {
+	// AllowCAPINotInstalled instructs CheckCAPIContract to tolerate management clusters without Cluster API installed yet.
+	AllowCAPINotInstalled bool
+
+	// AllowCAPIContract instructs CheckCAPIContract to tolerate management clusters with Cluster API with the given contract.
+	AllowCAPIContract string
+}
+
+// AllowCAPINotInstalled instructs CheckCAPIContract to tolerate management clusters without Cluster API installed yet.
+// NOTE: This allows clusterctl init to run on empty management clusters.
+type AllowCAPINotInstalled struct{}
+
+// Apply applies this configuration to the given CheckCAPIContractOptions.
+func (t AllowCAPINotInstalled) Apply(in *CheckCAPIContractOptions) {
+	in.AllowCAPINotInstalled = true
+}
+
+// AllowCAPIContract instructs CheckCAPIContract to tolerate management clusters with Cluster API with the given contract.
+// NOTE: This allows clusterctl upgrade to work on management clusters with old contract.
+type AllowCAPIContract struct {
+	Contract string
+}
+
+// Apply applies this configuration to the given CheckCAPIContractOptions.
+func (t AllowCAPIContract) Apply(in *CheckCAPIContractOptions) {
+	in.AllowCAPIContract = t.Contract
+}
+
 // InventoryClient exposes methods to interface with a cluster's provider inventory.
 type InventoryClient interface {
 	// EnsureCustomResourceDefinitions installs the CRD required for creating inventory items, if necessary.
@@ -69,6 +105,10 @@ type InventoryClient interface {
 
 	// GetManagementGroups returns the list of management groups defined in the management cluster.
 	GetManagementGroups() (ManagementGroupList, error)
+
+	// CheckCAPIContract checks the Cluster API version installed in the management cluster, and fails if this version
+	// does not match the current one supported by clusterctl.
+	CheckCAPIContract(...CheckCAPIContractOption) error
 }
 
 // inventoryClient implements InventoryClient.
@@ -188,14 +228,20 @@ func checkInventoryCRDs(proxy Proxy) (bool, error) {
 		return false, err
 	}
 
-	l := &clusterctlv1.ProviderList{}
-	if err = c.List(ctx, l); err == nil {
-		return true, nil
-	}
-	if !apimeta.IsNoMatchError(err) {
+	crd := &apiextensionsv1.CustomResourceDefinition{}
+	if err := c.Get(ctx, client.ObjectKey{Name: fmt.Sprintf("providers.%s", clusterctlv1.GroupVersion.Group)}, crd); err != nil {
+		if apierrors.IsNotFound(err) {
+			return false, nil
+		}
 		return false, errors.Wrap(err, "failed to check if the clusterctl inventory CRD exists")
 	}
-	return false, nil
+
+	for _, version := range crd.Spec.Versions {
+		if version.Name == clusterctlv1.GroupVersion.Version {
+			return true, nil
+		}
+	}
+	return true, errors.Errorf("clusterctl inventory CRD does not defines the %s version", clusterctlv1.GroupVersion.Version)
 }
 
 func (p *inventoryClient) createObj(o unstructured.Unstructured) error {
@@ -343,3 +389,33 @@ func (p *inventoryClient) GetDefaultProviderNamespace(provider string, providerT
 	// There is no provider or more than one namespace for this provider; in both cases, a default provider namespace cannot be decided.
 	return "", nil
 }
+
+func (p *inventoryClient) CheckCAPIContract(options ...CheckCAPIContractOption) error {
+	opt := &CheckCAPIContractOptions{}
+	for _, o := range options {
+		o.Apply(opt)
+	}
+
+	c, err := p.proxy.NewClient()
+	if err != nil {
+		return err
+	}
+
+	crd := &apiextensionsv1.CustomResourceDefinition{}
+	if err := c.Get(ctx, client.ObjectKey{Name: fmt.Sprintf("clusters.%s", clusterv1.GroupVersion.Group)}, crd); err != nil {
+		if opt.AllowCAPINotInstalled && apierrors.IsNotFound(err) {
+			return nil
+		}
+		return errors.Wrap(err, "failed to check Cluster API version")
+	}
+
+	for _, version := range crd.Spec.Versions {
+		if version.Storage {
+			if version.Name == clusterv1.GroupVersion.Version || version.Name == opt.AllowCAPIContract {
+				return nil
+			}
+			return errors.Errorf("this version of clusterctl could be used only with %q management clusters, %q detected", clusterv1.GroupVersion.Version, version.Name)
+		}
+	}
+	return errors.Errorf("failed to check Cluster API version")
+}

cmd/clusterctl/client/cluster/inventory_test.go

@@ -21,7 +21,7 @@ import (
 	"time"
 
 	. "github.com/onsi/gomega"
-
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -33,41 +33,46 @@ func fakePollImmediateWaiter(interval, timeout time.Duration, condition wait.Con
 	return nil
 }
 
-func Test_inventoryClient_EnsureCustomResourceDefinitions(t *testing.T) {
+func Test_inventoryClient_CheckInventoryCRDs(t *testing.T) {
 	type fields struct {
 		alreadyHasCRD bool
 	}
 	tests := []struct {
 		name    string
 		fields  fields
+		want    bool
 		wantErr bool
 	}{
 		{
 			name: "Has not CRD",
 			fields: fields{
 				alreadyHasCRD: false,
 			},
+			want:    false,
 			wantErr: false,
 		},
 		{
 			name: "Already has CRD",
 			fields: fields{
 				alreadyHasCRD: true,
 			},
+			want:    true,
 			wantErr: false,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			p := newInventoryClient(test.NewFakeProxy(), fakePollImmediateWaiter)
+			proxy := test.NewFakeProxy()
+			p := newInventoryClient(proxy, fakePollImmediateWaiter)
 			if tt.fields.alreadyHasCRD {
 				//forcing creation of metadata before test
 				g.Expect(p.EnsureCustomResourceDefinitions()).To(Succeed())
 			}
 
-			err := p.EnsureCustomResourceDefinitions()
+			res, err := checkInventoryCRDs(proxy)
+			g.Expect(res).To(Equal(tt.want))
 			if tt.wantErr {
 				g.Expect(err).To(HaveOccurred())
 			} else {
@@ -194,3 +199,139 @@ func Test_inventoryClient_Create(t *testing.T) {
 		})
 	}
 }
+
+func Test_CheckCAPIContract(t *testing.T) {
+	type args struct {
+		options []CheckCAPIContractOption
+	}
+	type fields struct {
+		proxy Proxy
+	}
+
+	tests := []struct {
+		name    string
+		fields  fields
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "Fails if Cluster API is not installed",
+			fields: fields{
+				proxy: test.NewFakeProxy().WithObjs(),
+			},
+			args:    args{},
+			wantErr: true,
+		},
+		{
+			name: "Pass if Cluster API is not installed, but this is explicitly tolerated",
+			fields: fields{
+				proxy: test.NewFakeProxy().WithObjs(),
+			},
+			args: args{
+				options: []CheckCAPIContractOption{AllowCAPINotInstalled{}},
+			},
+			wantErr: false,
+		},
+		{
+			name: "Pass when Cluster API with current contract is installed",
+			fields: fields{
+				proxy: test.NewFakeProxy().WithObjs(&apiextensionsv1.CustomResourceDefinition{
+					ObjectMeta: metav1.ObjectMeta{Name: "clusters.cluster.x-k8s.io"},
+					Spec: apiextensionsv1.CustomResourceDefinitionSpec{
+						Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
+							{
+								Name: test.PreviousCAPIContractNotSupported,
+							},
+							{
+								Name:    test.CurrentCAPIContract,
+								Storage: true,
+							},
+						},
+					},
+				}),
+			},
+			args:    args{},
+			wantErr: false,
+		},
+		{
+			name: "Fails when Cluster API with previous contract is installed",
+			fields: fields{
+				proxy: test.NewFakeProxy().WithObjs(&apiextensionsv1.CustomResourceDefinition{
+					ObjectMeta: metav1.ObjectMeta{Name: "clusters.cluster.x-k8s.io"},
+					Spec: apiextensionsv1.CustomResourceDefinitionSpec{
+						Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
+							{
+								Name:    test.PreviousCAPIContractNotSupported,
+								Storage: true,
+							},
+							{
+								Name: test.CurrentCAPIContract,
+							},
+						},
+					},
+				}),
+			},
+			args:    args{},
+			wantErr: true,
+		},
+		{
+			name: "Pass when Cluster API with previous contract is installed, but this is explicitly tolerated",
+			fields: fields{
+				proxy: test.NewFakeProxy().WithObjs(&apiextensionsv1.CustomResourceDefinition{
+					ObjectMeta: metav1.ObjectMeta{Name: "clusters.cluster.x-k8s.io"},
+					Spec: apiextensionsv1.CustomResourceDefinitionSpec{
+						Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
+							{
+								Name:    test.PreviousCAPIContractNotSupported,
+								Storage: true,
+							},
+							{
+								Name: test.CurrentCAPIContract,
+							},
+						},
+					},
+				}),
+			},
+			args: args{
+				options: []CheckCAPIContractOption{AllowCAPIContract{Contract: test.PreviousCAPIContractNotSupported}},
+			},
+			wantErr: false,
+		},
+		{
+			name: "Fails when Cluster API with next contract is installed",
+			fields: fields{
+				proxy: test.NewFakeProxy().WithObjs(&apiextensionsv1.CustomResourceDefinition{
+					ObjectMeta: metav1.ObjectMeta{Name: "clusters.cluster.x-k8s.io"},
+					Spec: apiextensionsv1.CustomResourceDefinitionSpec{
+						Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
+							{
+								Name: test.CurrentCAPIContract,
+							},
+							{
+								Name:    test.NextCAPIContractNotSupported,
+								Storage: true,
+							},
+						},
+					},
+				}),
+			},
+			args:    args{},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			p := &inventoryClient{
+				proxy: tt.fields.proxy,
+			}
+			err := p.CheckCAPIContract(tt.args.options...)
+			if tt.wantErr {
+				g.Expect(err).To(HaveOccurred())
+				return
+			}
+			g.Expect(err).NotTo(HaveOccurred())
+		})
+	}
+}

cmd/clusterctl/client/config.go

@@ -229,6 +229,11 @@ func (c *clusterctlClient) GetClusterTemplate(options GetClusterTemplateOptions)
 		return nil, err
 	}
 
+	// Ensure this command only runs against management clusters with the current Cluster API contract.
+	if err := cluster.ProviderInventory().CheckCAPIContract(); err != nil {
+		return nil, err
+	}
+
 	// If the option specifying the targetNamespace is empty, try to detect it.
 	if options.TargetNamespace == "" {
 		currentNamespace, err := cluster.Proxy().CurrentNamespace()

cmd/clusterctl/client/config_test.go

@@ -26,6 +26,7 @@ import (
 
 	. "github.com/onsi/gomega"
 	"github.com/pkg/errors"
+	"sigs.k8s.io/cluster-api/cmd/clusterctl/internal/test"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -469,7 +470,8 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
 
 	cluster1 := newFakeCluster(cluster.Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, config1).
 		WithProviderInventory(infraProviderConfig.Name(), infraProviderConfig.Type(), "v3.0.0", "foo", "bar").
-		WithObjs(configMap)
+		WithObjs(configMap).
+		WithObjs(test.FakeCAPISetupObjects()...)
 
 	client := newFakeClient(config1).
 		WithCluster(cluster1).

cmd/clusterctl/client/delete.go

@@ -66,6 +66,12 @@ func (c *clusterctlClient) Delete(options DeleteOptions) error {
 		return err
 	}
 
+	// Ensure this command only runs against management clusters with the current Cluster API contract.
+	if err := clusterClient.ProviderInventory().CheckCAPIContract(); err != nil {
+		return err
+	}
+
+	// Ensure the custom resource definitions required by clusterctl are in place.
 	if err := clusterClient.ProviderInventory().EnsureCustomResourceDefinitions(); err != nil {
 		return err
 	}

cmd/clusterctl/client/delete_test.go

@@ -21,7 +21,6 @@ import (
 	"testing"
 
 	. "github.com/onsi/gomega"
-
 	"k8s.io/apimachinery/pkg/util/sets"
 	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster"
@@ -207,6 +206,7 @@ func fakeClusterForDelete() *fakeClient {
 	cluster1.fakeProxy.WithProviderInventory(bootstrapProviderConfig.Name(), bootstrapProviderConfig.Type(), "v1.0.0", "capbpk-system", "")
 	cluster1.fakeProxy.WithProviderInventory(controlPlaneProviderConfig.Name(), controlPlaneProviderConfig.Type(), "v1.0.0", namespace, "")
 	cluster1.fakeProxy.WithProviderInventory(infraProviderConfig.Name(), infraProviderConfig.Type(), "v1.0.0", namespace, "")
+	cluster1.fakeProxy.WithFakeCAPISetup()
 
 	client := newFakeClient(config1).
 		// fake repository for capi, bootstrap, controlplane and infra provider (matching provider's config)