Merge pull request #452 from k8s-infra-cherrypick-robot/cherry-pick-4…

…51-to-release-0.9

[release-0.9] 🐛 Collect correct manifest config map per provider while performing upgrade

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