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
+ - infrastructure
  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{})
-}