pkg: Migrate from cluster-config-v1 to Infrastructure.config.openshif…

…t.io

Mostly, this is replacing cluster-config-v1 (which is deprecated [1])
with the Infrastructure config object.  As part of that, I'm dropping
UserTags support (see discussion in [2,3]).  I'm also replacing the
openshiftClusterID tag with a "kubernetes.io/cluster/{infraName}:
owned" tag [4,5].

The trip through clientcorev1.NewForConfig is because calling
RESTClientFor on the InClusterConfig raises:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1304/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-fc9948cc-b767n_cluster-image-registry-operator.log.gz | gunzip
  I0423 05:43:03.599910       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-g38fc8f9-dirty
  ...
  I0423 05:43:04.942285       1 bootstrap.go:38] generating registry custom resource
  E0423 05:43:04.942775       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: GroupVersion is required when initializing a RESTClient, requeuing

InClusterConfig does not set GroupVersion [6] and RESTClientFor
requires it [7] while UnversionedRESTClientFor can fill it in [8].
But UnversionedRESTClientFor leaves NegotiatedSerializer unset:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1305/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-66d8b56784-5wt9v_cluster-image-registry-operator.log.gz | gunzip
  I0423 09:21:42.998300       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-g64d5f62-dirty
  ...
  I0423 09:21:44.345677       1 bootstrap.go:38] generating registry custom resource
  E0423 09:21:44.346403       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: NegotiatedSerializer is required when initializing a RESTClient, requeuing

So I'm washing through clientcorev1.NewForConfig to get defaults for
both [9].

The RBAC change avoids:

  $ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-image-registry-operator/260/pull-ci-openshift-cluster-image-registry-operator-master-e2e-aws/1306/artifacts/e2e-aws/pods/openshift-image-registry_cluster-image-registry-operator-7f59899bf6-8c8x8_cluster-image-registry-operator.log.gz | gunzip | head -n7
  I0423 10:44:51.121540       1 main.go:18] Cluster Image Registry Operator Version: 4.0.0-142-gfa5b59b-dirty
  ...
  I0423 10:44:52.461691       1 bootstrap.go:38] generating registry custom resource
  E0423 10:44:52.464035       1 controller.go:235] unable to sync: unable to get storage configuration from cluster install config: infrastructures "cluster" is forbidden: User "system:serviceaccount:openshift-image-registry:cluster-image-registry-operator" cannot get resource "infrastructures" in API group "" at the cluster scope, requeuing

[1]: openshift/installer#680
[2]: openshift/api#231 (comment)
[3]: openshift/api#266
[4]: openshift/installer#1280
[5]: https://bugzilla.redhat.com/show_bug.cgi?id=1685089#c11
[6]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L422-L428
[7]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L269-L270
[8]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/rest/config.go#L331-L333
[9]: https://github.com/kubernetes/client-go/blob/59781b88d0faa9d51be48b4ca49eb08e05246de4/kubernetes/typed/core/v1/core_client.go#L144-L155

manifests/02-rbac.yaml

@@ -20,7 +20,7 @@ rules:
 - apiGroups:
   - config.openshift.io
   resources:
-  - clusterversions
+  - infrastructures
   verbs:
   - get
 - apiGroups:

pkg/clusterconfig/clusterconfig.go

@@ -1,27 +1,25 @@
 package clusterconfig
 
 import (
+	"context"
 	"fmt"
-	"strings"
 	"time"
 
-	installer "github.com/openshift/installer/pkg/types"
-
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/client-go/rest"
 
 	coreset "k8s.io/client-go/kubernetes/typed/core/v1"
 
+	configv1 "github.com/openshift/api/config/v1"
+	osclientset "github.com/openshift/client-go/config/clientset/versioned"
 	imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1"
 	regopclient "github.com/openshift/cluster-image-registry-operator/pkg/client"
 )
 
 const (
-	installerConfigNamespace = "kube-system"
-	installerConfigName      = "cluster-config-v1"
-	cloudCredentialsName     = "installer-cloud-credentials"
+	cloudCredentialsName = "installer-cloud-credentials"
 )
 
 type StorageType string
@@ -68,47 +66,23 @@ func getCoreClient() (*coreset.CoreV1Client, error) {
 	return client, nil
 }
 
-func GetInstallConfig() (*installer.InstallConfig, error) {
-	client, err := getCoreClient()
-	if err != nil {
-		return nil, err
-	}
-
-	cm, err := client.ConfigMaps(installerConfigNamespace).Get(installerConfigName, metav1.GetOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("unable to read cluster install configuration: %v", err)
-	}
-
-	installConfig := &installer.InstallConfig{}
-	if err := yaml.NewYAMLOrJSONDecoder(strings.NewReader(cm.Data["install-config"]), 100).Decode(installConfig); err != nil {
-		return nil, fmt.Errorf("unable to decode cluster install configuration: %v", err)
-	}
-
-	return installConfig, nil
-}
-
-func GetAWSConfig(listers *regopclient.Listers) (*Config, error) {
+func GetAWSConfig(ctx context.Context, client rest.Interface, listers *regopclient.Listers) (*Config, error) {
 	cfg := &Config{}
 
-	installConfig, err := GetInstallConfig()
+	infra, err := osclientset.New(client).ConfigV1().Infrastructures().Get("cluster", metav1.GetOptions{})
 	if err != nil {
 		return nil, err
 	}
 
-	if installConfig.Platform.AWS != nil {
-		cfg.Storage.S3.Region = installConfig.Platform.AWS.Region
-	}
-
-	client, err := getCoreClient()
-	if err != nil {
-		return nil, err
+	if infra.Status.Platform == configv1.AWSPlatformType {
+		cfg.Storage.S3.Region = "us-east-1" // FIXME: installConfig.Platform.AWS.Region
 	}
 
 	// Look for a user defined secret to get the AWS credentials from first
 	sec, err := listers.Secrets.Get(imageregistryv1.ImageRegistryPrivateConfigurationUser)
 	if err != nil && errors.IsNotFound(err) {
 		pollErr := wait.PollImmediate(1*time.Second, 5*time.Minute, func() (stop bool, err error) {
-			sec, err = client.Secrets(imageregistryv1.ImageRegistryOperatorNamespace).Get(cloudCredentialsName, metav1.GetOptions{})
+			sec, err = coreset.New(client).Secrets(imageregistryv1.ImageRegistryOperatorNamespace).Get(cloudCredentialsName, metav1.GetOptions{})
 			if err != nil {
 				if errors.IsNotFound(err) {
 					return false, nil

pkg/storage/s3/s3.go

@@ -1,6 +1,7 @@
 package s3
 
 import (
+	"context"
 	"fmt"
 	"reflect"
 	"strings"
@@ -12,11 +13,14 @@ import (
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/s3"
 
+	configv1 "github.com/openshift/api/config/v1"
 	operatorapi "github.com/openshift/api/operator/v1"
+	osclientset "github.com/openshift/client-go/config/clientset/versioned"
 
 	corev1 "k8s.io/api/core/v1"
-
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	clientcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 
 	imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1"
 	regopclient "github.com/openshift/cluster-image-registry-operator/pkg/client"
@@ -46,11 +50,23 @@ func NewDriver(c *imageregistryv1.ImageRegistryConfigStorageS3, listers *regopcl
 // getS3Service returns a client that allows us to interact
 // with the aws S3 service
 func (d *driver) getS3Service() (*s3.S3, error) {
+	ctx := context.TODO()
+
 	if s3Service != nil {
 		return s3Service, nil
 	}
 
-	cfg, err := clusterconfig.GetAWSConfig(d.Listers)
+	config, err := regopclient.GetConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	client, err := clientcorev1.NewForConfig(config)
+	if err != nil {
+		return nil, err
+	}
+
+	cfg, err := clusterconfig.GetAWSConfig(ctx, client.RESTClient(), d.Listers)
 	if err != nil {
 		return nil, err
 	}
@@ -161,7 +177,19 @@ func (d *driver) Volumes() ([]corev1.Volume, []corev1.VolumeMount, error) {
 
 // Secrets returns a map of the storage access secrets.
 func (d *driver) Secrets() (map[string]string, error) {
-	cfg, err := clusterconfig.GetAWSConfig(d.Listers)
+	ctx := context.TODO()
+
+	config, err := regopclient.GetConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	client, err := clientcorev1.NewForConfig(config)
+	if err != nil {
+		return nil, err
+	}
+
+	cfg, err := clusterconfig.GetAWSConfig(ctx, client.RESTClient(), d.Listers)
 	if err != nil {
 		return nil, err
 	}
@@ -234,12 +262,17 @@ func (d *driver) CreateStorage(cr *imageregistryv1.Config) error {
 		return err
 	}
 
-	ic, err := clusterconfig.GetInstallConfig()
+	config, err := regopclient.GetConfig()
+	if err != nil {
+		return err
+	}
+
+	client, err := clientcorev1.NewForConfig(config)
 	if err != nil {
 		return err
 	}
 
-	cv, err := util.GetClusterVersionConfig()
+	infra, err := osclientset.New(client.RESTClient()).ConfigV1().Infrastructures().Get("cluster", metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
@@ -277,7 +310,7 @@ func (d *driver) CreateStorage(cr *imageregistryv1.Config) error {
 		for i := 0; i < 5000; i++ {
 			// If the bucket name is blank, let's generate one
 			if len(d.Config.Bucket) == 0 {
-				d.Config.Bucket = fmt.Sprintf("%s-%s-%s-%s", imageregistryv1.ImageRegistryName, d.Config.Region, strings.Replace(string(cv.Spec.ClusterID), "-", "", -1), strings.Replace(string(uuid.NewUUID()), "-", "", -1))[0:62]
+				d.Config.Bucket = fmt.Sprintf("%s-%s-%s-%s", imageregistryv1.ImageRegistryName, d.Config.Region, strings.Replace(infra.Status.InfrastructureName, "-", "", -1), strings.Replace(string(uuid.NewUUID()), "-", "", -1))[0:62]
 				generatedName = true
 			}
 
@@ -326,20 +359,18 @@ func (d *driver) CreateStorage(cr *imageregistryv1.Config) error {
 		return err
 	}
 
-	// Tag the bucket with the openshiftClusterID
-	// along with any user defined tags from the cluster configuration
+	// Tag the bucket with kubernetes.io/cluster/{infrastructureName}
 	if cr.Status.StorageManaged {
-		if ic.Platform.AWS != nil {
-			var tagSet []*s3.Tag
-			tagSet = append(tagSet, &s3.Tag{Key: aws.String("openshiftClusterID"), Value: aws.String(string(cv.Spec.ClusterID))})
-			for k, v := range ic.Platform.AWS.UserTags {
-				tagSet = append(tagSet, &s3.Tag{Key: aws.String(k), Value: aws.String(v)})
-			}
-
+		if infra.Status.Platform == configv1.AWSPlatformType {
 			_, err := svc.PutBucketTagging(&s3.PutBucketTaggingInput{
 				Bucket: aws.String(d.Config.Bucket),
 				Tagging: &s3.Tagging{
-					TagSet: tagSet,
+					TagSet: []*s3.Tag{
+						{
+							Key:   aws.String("kubernetes.io/cluster/" + infra.Status.InfrastructureName),
+							Value: aws.String("owned"),
+						},
+					},
 				},
 			})
 			if err != nil {
@@ -349,7 +380,7 @@ func (d *driver) CreateStorage(cr *imageregistryv1.Config) error {
 					util.UpdateCondition(cr, imageregistryv1.StorageTagged, operatorapi.ConditionFalse, "Unknown Error Occurred", err.Error())
 				}
 			} else {
-				util.UpdateCondition(cr, imageregistryv1.StorageTagged, operatorapi.ConditionTrue, "Tagging Successful", "UserTags were successfully applied to the S3 bucket")
+				util.UpdateCondition(cr, imageregistryv1.StorageTagged, operatorapi.ConditionTrue, "Tagging Successful", "Tags were successfully applied to the S3 bucket")
 			}
 		}
 	}
@@ -487,7 +518,19 @@ func (d *driver) RemoveStorage(cr *imageregistryv1.Config) (bool, error) {
 }
 
 func (d *driver) CompleteConfiguration(cr *imageregistryv1.Config) error {
-	cfg, err := clusterconfig.GetAWSConfig(d.Listers)
+	ctx := context.TODO()
+
+	config, err := regopclient.GetConfig()
+	if err != nil {
+		return err
+	}
+
+	client, err := clientcorev1.NewForConfig(config)
+	if err != nil {
+		return err
+	}
+
+	cfg, err := clusterconfig.GetAWSConfig(ctx, client.RESTClient(), d.Listers)
 	if err != nil {
 		return err
 	}

pkg/storage/storage.go

@@ -4,10 +4,13 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clientcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 
+	configv1 "github.com/openshift/api/config/v1"
+	osclientset "github.com/openshift/client-go/config/clientset/versioned"
 	imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1"
 	regopclient "github.com/openshift/cluster-image-registry-operator/pkg/client"
-	"github.com/openshift/cluster-image-registry-operator/pkg/clusterconfig"
 	"github.com/openshift/cluster-image-registry-operator/pkg/storage/emptydir"
 	"github.com/openshift/cluster-image-registry-operator/pkg/storage/pvc"
 	"github.com/openshift/cluster-image-registry-operator/pkg/storage/s3"
@@ -79,17 +82,27 @@ func NewDriver(cfg *imageregistryv1.ImageRegistryConfigStorage, listers *regopcl
 func getPlatformStorage() (imageregistryv1.ImageRegistryConfigStorage, error) {
 	var cfg imageregistryv1.ImageRegistryConfigStorage
 
-	installConfig, err := clusterconfig.GetInstallConfig()
+	config, err := regopclient.GetConfig()
+	if err != nil {
+		return cfg, err
+	}
+
+	client, err := clientcorev1.NewForConfig(config)
+	if err != nil {
+		return cfg, err
+	}
+
+	infra, err := osclientset.New(client.RESTClient()).ConfigV1().Infrastructures().Get("cluster", metav1.GetOptions{})
 	if err != nil {
 		return cfg, err
 	}
 
 	switch {
-	case installConfig.Platform.Libvirt != nil:
+	case infra.Status.Platform == configv1.LibvirtPlatformType:
 		cfg.EmptyDir = &imageregistryv1.ImageRegistryConfigStorageEmptyDir{}
-	case installConfig.Platform.AWS != nil:
+	case infra.Status.Platform == configv1.AWSPlatformType:
 		cfg.S3 = &imageregistryv1.ImageRegistryConfigStorageS3{}
-	case installConfig.Platform.OpenStack != nil:
+	case infra.Status.Platform == configv1.OpenStackPlatformType:
 		// TODO(flaper87): This should be switch to swift as soon as support for
 		// it is complete. Using Emptydir for now so that OpenStack deployments
 		// (and work) can move forward for now. Not production ready!

pkg/storage/util/util.go

@@ -14,14 +14,9 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	metaapi "k8s.io/apimachinery/pkg/apis/meta/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	coreset "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/util/retry"
-
-	configv1 "github.com/openshift/api/config/v1"
-
-	configv1client "github.com/openshift/client-go/config/clientset/versioned"
 )
 
 // UpdateCondition will update or add the provided condition.
@@ -113,16 +108,3 @@ func CreateOrUpdateSecret(name string, namespace string, data map[string]string)
 
 	return updatedSecret, err
 }
-
-func GetClusterVersionConfig() (*configv1.ClusterVersion, error) {
-	kubeconfig, err := regopclient.GetConfig()
-	if err != nil {
-		return nil, err
-	}
-
-	client, err := configv1client.NewForConfig(kubeconfig)
-	if err != nil {
-		return nil, err
-	}
-	return client.ConfigV1().ClusterVersions().Get("version", metav1.GetOptions{})
-}