From 4b2bbe5d61878a6e0387563f339b439cb82c406d Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 22 Apr 2019 16:26:31 -0700 Subject: [PATCH] pkg: Migrate from cluster-config-v1 to Infrastructure.config.openshift.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]: https://github.com/openshift/installer/issues/680 [2]: https://github.com/openshift/api/pull/231#discussion_r264390588 [3]: https://github.com/openshift/api/pull/266 [4]: https://github.com/openshift/installer/pull/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 | 2 +- pkg/clusterconfig/clusterconfig.go | 46 ++++------------- pkg/storage/s3/s3.go | 79 +++++++++++++++++++++++------- pkg/storage/storage.go | 23 +++++++-- pkg/storage/util/util.go | 18 ------- 5 files changed, 90 insertions(+), 78 deletions(-) diff --git a/manifests/02-rbac.yaml b/manifests/02-rbac.yaml index 8de5b91720..138933a03e 100644 --- a/manifests/02-rbac.yaml +++ b/manifests/02-rbac.yaml @@ -20,7 +20,7 @@ rules: - apiGroups: - config.openshift.io resources: - - clusterversions + - infrastructure verbs: - get - apiGroups: diff --git a/pkg/clusterconfig/clusterconfig.go b/pkg/clusterconfig/clusterconfig.go index ba0d175099..0d0782838b 100644 --- a/pkg/clusterconfig/clusterconfig.go +++ b/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 diff --git a/pkg/storage/s3/s3.go b/pkg/storage/s3/s3.go index 701b83283b..aea725b07f 100644 --- a/pkg/storage/s3/s3.go +++ b/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 } diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 4d2d735b57..41e94214ce 100644 --- a/pkg/storage/storage.go +++ b/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! diff --git a/pkg/storage/util/util.go b/pkg/storage/util/util.go index e50073bbd8..cb578e9c56 100644 --- a/pkg/storage/util/util.go +++ b/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{}) -}