Merge pull request #451 from Danil-Grigorev/inject-repo-for-upgrade-validation

🐛 Collect correct manifest config map per provider while performing upgrade

Author: k8s-ci-robot

internal/controller/manifests_downloader.go

@@ -66,7 +66,7 @@ func (p *phaseReconciler) downloadManifests(ctx context.Context) (reconcile.Resu
 		MatchLabels: p.prepareConfigMapLabels(),
 	}
 
-	exists, err := p.checkConfigMapExists(ctx, labelSelector)
+	exists, err := p.checkConfigMapExists(ctx, labelSelector, p.provider.GetNamespace())
 	if err != nil {
 		return reconcile.Result{}, wrapPhaseError(err, "failed to check that config map with manifests exists", operatorv1.ProviderInstalledCondition)
 	}
@@ -123,11 +123,11 @@ func (p *phaseReconciler) downloadManifests(ctx context.Context) (reconcile.Resu
 }
 
 // checkConfigMapExists checks if a config map exists in Kubernetes with the given LabelSelector.
-func (p *phaseReconciler) checkConfigMapExists(ctx context.Context, labelSelector metav1.LabelSelector) (bool, error) {
+func (p *phaseReconciler) checkConfigMapExists(ctx context.Context, labelSelector metav1.LabelSelector, namespace string) (bool, error) {
 	labelSet := labels.Set(labelSelector.MatchLabels)
 	listOpts := []client.ListOption{
 		client.MatchingLabelsSelector{Selector: labels.SelectorFromSet(labelSet)},
-		client.InNamespace(p.provider.GetNamespace()),
+		client.InNamespace(namespace),
 	}
 
 	var configMapList corev1.ConfigMapList
@@ -145,12 +145,7 @@ func (p *phaseReconciler) checkConfigMapExists(ctx context.Context, labelSelecto
 
 // prepareConfigMapLabels returns labels that identify a config map with downloaded manifests.
 func (p *phaseReconciler) prepareConfigMapLabels() map[string]string {
-	return map[string]string{
-		configMapVersionLabel: p.provider.GetSpec().Version,
-		configMapTypeLabel:    p.provider.GetType(),
-		configMapNameLabel:    p.provider.GetName(),
-		operatorManagedLabel:  "true",
-	}
+	return providerLabels(p.provider)
 }
 
 // createManifestsConfigMap creates a config map with downloaded manifests.
@@ -210,6 +205,27 @@ func (p *phaseReconciler) createManifestsConfigMap(ctx context.Context, metadata
 	return nil
 }
 
+func providerLabelSelector(provider operatorv1.GenericProvider) *metav1.LabelSelector {
+	// Replace label selector if user wants to use custom config map
+	if provider.GetSpec().FetchConfig != nil && provider.GetSpec().FetchConfig.Selector != nil {
+		return provider.GetSpec().FetchConfig.Selector
+	}
+
+	return &metav1.LabelSelector{
+		MatchLabels: providerLabels(provider),
+	}
+}
+
+// prepareConfigMapLabels returns default set of labels that identify a config map with downloaded manifests.
+func providerLabels(provider operatorv1.GenericProvider) map[string]string {
+	return map[string]string{
+		configMapVersionLabel: provider.GetSpec().Version,
+		configMapTypeLabel:    provider.GetType(),
+		configMapNameLabel:    provider.GetName(),
+		operatorManagedLabel:  "true",
+	}
+}
+
 // needToCompress checks whether the input data exceeds the maximum configmap
 // size limit and returns whether it should be compressed.
 func needToCompress(bs ...[]byte) bool {

internal/controller/manifests_downloader_test.go

@@ -62,7 +62,7 @@ func TestManifestsDownloader(t *testing.T) {
 		MatchLabels: p.prepareConfigMapLabels(),
 	}
 
-	exists, err := p.checkConfigMapExists(ctx, labelSelector)
+	exists, err := p.checkConfigMapExists(ctx, labelSelector, p.provider.GetNamespace())
 	g.Expect(err).ToNot(HaveOccurred())
 
 	g.Expect(exists).To(BeTrue())

internal/controller/phases.go

@@ -48,7 +48,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
-const metadataFile = "metadata.yaml"
+const (
+	metadataFile = "metadata.yaml"
+)
 
 // phaseReconciler holds all required information for interacting with clusterctl code and
 // helps to iterate through provider reconciliation phases.
@@ -574,7 +576,32 @@ func clusterctlProviderName(provider operatorv1.GenericProvider) client.ObjectKe
 }
 
 func (p *phaseReconciler) repositoryProxy(ctx context.Context, provider configclient.Provider, configClient configclient.Client, options ...repository.Option) (repository.Client, error) {
-	cl, err := repository.New(ctx, provider, configClient, append([]repository.Option{repository.InjectRepository(p.repo)}, options...)...)
+	injectRepo := p.repo
+
+	if !provider.SameAs(p.providerConfig) {
+		genericProvider, err := util.GetGenericProvider(ctx, p.ctrlClient, provider)
+		if err != nil {
+			return nil, wrapPhaseError(err, "unable to find generic provider for configclient "+string(provider.Type())+": "+provider.Name(), operatorv1.ProviderUpgradedCondition)
+		}
+
+		if exists, err := p.checkConfigMapExists(ctx, *providerLabelSelector(genericProvider), genericProvider.GetNamespace()); err != nil {
+			provider := client.ObjectKeyFromObject(genericProvider)
+			return nil, wrapPhaseError(err, "failed to check the config map repository existence for provider "+provider.String(), operatorv1.ProviderUpgradedCondition)
+		} else if !exists {
+			provider := client.ObjectKeyFromObject(genericProvider)
+			return nil, wrapPhaseError(fmt.Errorf("config map not found"), "config map repository required for validation does not exist yet for provider "+provider.String(), operatorv1.ProviderUpgradedCondition)
+		}
+
+		repo, err := p.configmapRepository(ctx, providerLabelSelector(genericProvider), genericProvider.GetNamespace(), "")
+		if err != nil {
+			provider := client.ObjectKeyFromObject(genericProvider)
+			return nil, wrapPhaseError(err, "failed to load the repository for provider "+provider.String(), operatorv1.ProviderUpgradedCondition)
+		}
+
+		injectRepo = repo
+	}
+
+	cl, err := repository.New(ctx, provider, configClient, append([]repository.Option{repository.InjectRepository(injectRepo)}, options...)...)
 	if err != nil {
 		return nil, err
 	}

internal/controller/phases_test.go

@@ -18,6 +18,7 @@ package controller
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -26,6 +27,7 @@ import (
 	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	configclient "sigs.k8s.io/cluster-api/cmd/clusterctl/client/config"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha2"
@@ -412,6 +414,214 @@ metadata:
 	}
 }
 
+func TestRepositoryProxy(t *testing.T) {
+	coreProvider := configclient.NewProvider("cluster-api", "https://github.com/kubernetes-sigs/cluster-api/releases/latest/core-components.yaml", clusterctlv1.CoreProviderType)
+	awsProvider := configclient.NewProvider("aws", "https://github.com/kubernetes-sigs/cluster-api-provider-aws/releases/v1.4.1/infrastructure-components.yaml", clusterctlv1.InfrastructureProviderType)
+
+	provider := &operatorv1.InfrastructureProvider{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "aws",
+			Namespace: "ns1",
+		},
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "InfrastructureProvider",
+			APIVersion: "operator.cluster.x-k8s.io/v1alpha2",
+		},
+		Spec: operatorv1.InfrastructureProviderSpec{
+			ProviderSpec: operatorv1.ProviderSpec{
+				Version: "v2.3.5",
+				FetchConfig: &operatorv1.FetchConfiguration{
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{"provider-components": "aws"},
+					},
+				},
+			},
+		},
+	}
+
+	core := &operatorv1.CoreProvider{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "cluster-api",
+			Namespace: "default",
+		},
+		Spec: operatorv1.CoreProviderSpec{
+			ProviderSpec: operatorv1.ProviderSpec{
+				Version: testCurrentVersion,
+			},
+		},
+	}
+
+	awsMetadata := `
+apiVersion: clusterctl.cluster.x-k8s.io/v1alpha3
+releaseSeries:
+  - major: 2
+    minor: 4
+    contract: v1beta1
+  - major: 2
+    minor: 3
+    contract: v1beta1`
+
+	awsMetaReleaseSeries := []clusterctlv1.ReleaseSeries{
+		{
+			Major:    2,
+			Minor:    4,
+			Contract: "v1beta1",
+		}, {
+			Major:    2,
+			Minor:    3,
+			Contract: "v1beta1",
+		},
+	}
+
+	metadata := `
+apiVersion: clusterctl.cluster.x-k8s.io/v1alpha3
+releaseSeries:
+  - major: 0
+    minor: 4
+    contract: v1alpha4
+  - major: 0
+    minor: 3
+    contract: v1alpha3`
+
+	metaReleaseSeries := []clusterctlv1.ReleaseSeries{{
+		Major:    0,
+		Minor:    4,
+		Contract: "v1alpha4",
+	}, {
+		Major:    0,
+		Minor:    3,
+		Contract: "v1alpha3",
+	}}
+
+	tests := []struct {
+		name               string
+		configMaps         []corev1.ConfigMap
+		genericProviders   []client.Object
+		provider           configclient.Provider
+		defaultRepository  bool
+		wantMetadataSeries []clusterctlv1.ReleaseSeries
+		wantErr            string
+		metadataErr        string
+		wantDefaultVersion string
+	}{
+		{
+			name:               "missing configmaps",
+			provider:           coreProvider,
+			wantDefaultVersion: testCurrentVersion,
+			genericProviders:   []client.Object{core, provider},
+			metadataErr:        "failed to read \"metadata.yaml\" from the repository for provider \"cluster-api\": unable to get files for version v0.4.2",
+		},
+		{
+			name:               "correct configmap with data",
+			genericProviders:   []client.Object{core, provider},
+			provider:           coreProvider,
+			defaultRepository:  true,
+			wantDefaultVersion: testCurrentVersion,
+			wantMetadataSeries: metaReleaseSeries,
+			configMaps: []corev1.ConfigMap{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "ConfigMap",
+						APIVersion: "v1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      testCurrentVersion,
+						Namespace: "default",
+						Labels: map[string]string{
+							configMapVersionLabel: testCurrentVersion,
+							configMapTypeLabel:    core.GetType(),
+							configMapNameLabel:    core.GetName(),
+							operatorManagedLabel:  "true",
+						},
+					},
+					Data: map[string]string{"metadata": metadata, "components": ""},
+				},
+			},
+		},
+		{
+			name:             "upgrade required validation of another provider missing config map",
+			genericProviders: []client.Object{core, provider},
+			provider:         awsProvider,
+			wantErr:          wrapPhaseError(fmt.Errorf("config map not found"), "config map repository required for validation does not exist yet for provider ns1/aws", operatorv1.ProviderUpgradedCondition).Error(),
+		},
+		{
+			name:             "updgrade requested an unknown provider for the operator",
+			genericProviders: []client.Object{core},
+			provider:         awsProvider,
+			wantErr:          wrapPhaseError(fmt.Errorf("unable to find provider manifest with name aws"), "unable to find generic provider for configclient InfrastructureProvider: aws", operatorv1.ProviderUpgradedCondition).Error(),
+		},
+		{
+			name:               "upgrade required validation of another provider metadata succeeds",
+			genericProviders:   []client.Object{core, provider},
+			provider:           awsProvider,
+			wantDefaultVersion: "v2.3.5",
+			wantMetadataSeries: awsMetaReleaseSeries,
+			configMaps: []corev1.ConfigMap{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "ConfigMap",
+						APIVersion: "v1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "v2.3.5",
+						Namespace: provider.Namespace,
+						Labels:    map[string]string{"provider-components": "aws"},
+					},
+					Data: map[string]string{"metadata": awsMetadata, "components": ""},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			fakeclient := fake.NewClientBuilder().WithScheme(setupScheme()).WithObjects(tt.genericProviders...).Build()
+			p := &phaseReconciler{
+				ctrlClient:     fakeclient,
+				providerConfig: coreProvider,
+				repo:           repository.NewMemoryRepository(),
+				provider:       core,
+			}
+
+			for i := range tt.configMaps {
+				g.Expect(fakeclient.Create(ctx, &tt.configMaps[i])).To(Succeed())
+			}
+			if tt.defaultRepository {
+				var err error
+				p.repo, err = p.configmapRepository(ctx, &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						operatorManagedLabel: "true",
+					},
+				}, "default", "")
+				g.Expect(err).To(Succeed())
+			}
+
+			cl, err := configclient.New(ctx, "")
+			g.Expect(err).To(Succeed())
+
+			got, err := p.repositoryProxy(ctx, tt.provider, cl)
+			if len(tt.wantErr) > 0 {
+				g.Expect(err).Should(MatchError(tt.wantErr))
+				return
+			}
+			g.Expect(err).To(Succeed())
+
+			meta := got.Metadata(tt.wantDefaultVersion)
+			metadataData, err := meta.Get(ctx)
+			if len(tt.metadataErr) > 0 {
+				g.Expect(err).Should(MatchError(tt.metadataErr))
+				return
+			}
+			g.Expect(err).To(Succeed())
+			g.Expect(metadataData.ReleaseSeries).To(Equal(tt.wantMetadataSeries))
+
+			g.Expect(got.DefaultVersion()).To(Equal(tt.wantDefaultVersion))
+		})
+	}
+}
+
 func TestRepositoryFactory(t *testing.T) {
 	testCases := []struct {
 		name          string