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

⚠️ Clustectl v1alpha3 should block on v1alpha4 clusters #4265

Merged
Show file tree
Hide file tree
Changes from all 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
90 changes: 83 additions & 7 deletions cmd/clusterctl/client/cluster/inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
}
149 changes: 145 additions & 4 deletions cmd/clusterctl/client/cluster/inventory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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())
})
}
}
5 changes: 5 additions & 0 deletions cmd/clusterctl/client/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion cmd/clusterctl/client/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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).
Expand Down
6 changes: 6 additions & 0 deletions cmd/clusterctl/client/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/clusterctl/client/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
Loading