From 59b66a9d6c8284726418f16f531b63f2f5bd9776 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Fri, 17 Jan 2020 14:08:36 +0100 Subject: [PATCH 1/2] Allow to check Swift availability This patch adds a function that checks if Swift service is enabled. To do so it reuses the existing getSwiftClient function, that has this check. --- pkg/storage/swift/swift.go | 22 ++++++++++++++++++---- pkg/storage/swift/swift_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/pkg/storage/swift/swift.go b/pkg/storage/swift/swift.go index 3fb05915f9..c8764b67b7 100644 --- a/pkg/storage/swift/swift.go +++ b/pkg/storage/swift/swift.go @@ -52,6 +52,20 @@ func replaceEmpty(a string, b string) string { return a } +// IsSwiftEnabled checks if Swift service is available for OpenStack platform +func IsSwiftEnabled(listers *regopclient.Listers) (bool, error) { + driver := NewDriver(&imageregistryv1.ImageRegistryConfigStorageSwift{}, listers) + _, err := driver.getSwiftClient() + if err != nil { + // ErrEndpointNotFound means that Swift is not available + if _, ok := err.(*gophercloud.ErrEndpointNotFound); ok { + return false, nil + } + return false, err + } + return true, nil +} + // GetConfig reads credentials func GetConfig(listers *regopclient.Listers) (*Swift, error) { cfg := &Swift{} @@ -129,7 +143,7 @@ func GetConfig(listers *regopclient.Listers) (*Swift, error) { } // getSwiftClient returns a client that allows to interact with the OpenStack Swift service -func (d *driver) getSwiftClient(cr *imageregistryv1.Config) (*gophercloud.ServiceClient, error) { +func (d *driver) getSwiftClient() (*gophercloud.ServiceClient, error) { cfg, err := GetConfig(d.Listers) if err != nil { return nil, err @@ -288,7 +302,7 @@ func (d *driver) containerExists(client *gophercloud.ServiceClient, containerNam } func (d *driver) StorageExists(cr *imageregistryv1.Config) (bool, error) { - client, err := d.getSwiftClient(cr) + client, err := d.getSwiftClient() if err != nil { util.UpdateCondition(cr, defaults.StorageExists, operatorapi.ConditionUnknown, "Could not connect to registry storage", err.Error()) return false, err @@ -318,7 +332,7 @@ func (d *driver) StorageChanged(cr *imageregistryv1.Config) bool { } func (d *driver) CreateStorage(cr *imageregistryv1.Config) error { - client, err := d.getSwiftClient(cr) + client, err := d.getSwiftClient() if err != nil { util.UpdateCondition(cr, defaults.StorageExists, operatorapi.ConditionFalse, err.Error(), err.Error()) return err @@ -396,7 +410,7 @@ func (d *driver) RemoveStorage(cr *imageregistryv1.Config) (bool, error) { return false, nil } - client, err := d.getSwiftClient(cr) + client, err := d.getSwiftClient() if err != nil { return false, err } diff --git a/pkg/storage/swift/swift_test.go b/pkg/storage/swift/swift_test.go index 4994dad912..a8231ea151 100644 --- a/pkg/storage/swift/swift_test.go +++ b/pkg/storage/swift/swift_test.go @@ -5,6 +5,7 @@ import ( "net/http" "testing" + "github.com/gophercloud/gophercloud" th "github.com/gophercloud/gophercloud/testhelper" corev1 "k8s.io/api/core/v1" @@ -609,3 +610,33 @@ func TestSwiftEndpointTypeObjectStore(t *testing.T) { th.AssertNoErr(t, err) th.AssertEquals(t, true, res) } + +func TestSwiftIsNotAvailable(t *testing.T) { + th.SetupHTTP() + defer th.TeardownHTTP() + // Swift endpoint is not registered + handleAuthentication(t, "INVALID") + + th.Mux.HandleFunc("/"+container, func(w http.ResponseWriter, r *http.Request) { + th.TestMethod(t, r, "HEAD") + th.TestHeader(t, r, "Accept", "application/json") + w.Header().Set("Accept-Ranges", "bytes") + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.Header().Set("Date", "Wed, 17 Aug 2016 19:25:43 GMT") + w.Header().Set("X-Container-Bytes-Used", "100") + w.Header().Set("X-Container-Object-Count", "4") + w.Header().Set("X-Container-Read", "test") + w.Header().Set("X-Container-Write", "test2,user4") + w.Header().Set("X-Timestamp", "1471298837.95721") + w.Header().Set("X-Trans-Id", "tx554ed59667a64c61866f1-0057b4ba37") + w.Header().Set("X-Storage-Policy", "test_policy") + w.WriteHeader(http.StatusNoContent) + }) + + d, _ := mockConfig(false, th.Endpoint()+"v3", MockUPISecretNamespaceLister{}) + + _, err := d.getSwiftClient() + // if Swift endpoint is not registered, getSwiftClient should return ErrEndpointNotFound + _, ok := err.(*gophercloud.ErrEndpointNotFound) + th.AssertEquals(t, true, ok) +} From 6c8f7f826455c40721f78df6e6180da68d9d4743 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Thu, 16 Jan 2020 12:32:49 +0100 Subject: [PATCH 2/2] Allows to use RWO PVC backend for OpenStack This commit allows to use RWO PVC backend for OpenStack if Swift is not available. --- defaults/defaults.go | 3 ++ pkg/operator/bootstrap.go | 77 ++++++++++++++++++++++++++++++++++++++- pkg/storage/pvc/pvc.go | 8 ++-- pkg/storage/storage.go | 13 ++++++- 4 files changed, 95 insertions(+), 6 deletions(-) diff --git a/defaults/defaults.go b/defaults/defaults.go index de5d9608e5..cec254c2dd 100644 --- a/defaults/defaults.go +++ b/defaults/defaults.go @@ -8,6 +8,9 @@ const ( // ImageRegistryName is the name of the image-registry workload resource (deployment) ImageRegistryName = "image-registry" + // PVCImageRegistryName is the default name of the claim provisioned for PVC backend + PVCImageRegistryName = "image-registry-storage" + // ImageRegistryResourceName is the name of the image registry config instance ImageRegistryResourceName = "cluster" diff --git a/pkg/operator/bootstrap.go b/pkg/operator/bootstrap.go index a31de828f3..6766417108 100644 --- a/pkg/operator/bootstrap.go +++ b/pkg/operator/bootstrap.go @@ -7,12 +7,18 @@ import ( "github.com/openshift/cluster-image-registry-operator/defaults" "github.com/openshift/cluster-image-registry-operator/pkg/parameters" "github.com/openshift/cluster-image-registry-operator/pkg/storage" + "github.com/openshift/cluster-image-registry-operator/pkg/storage/pvc" + "github.com/openshift/cluster-image-registry-operator/pkg/storage/swift" + "github.com/openshift/cluster-image-registry-operator/pkg/storage/util" + configapiv1 "github.com/openshift/api/config/v1" imageregistryv1 "github.com/openshift/api/imageregistry/v1" operatorapi "github.com/openshift/api/operator/v1" regopset "github.com/openshift/client-go/imageregistry/clientset/versioned/typed/imageregistry/v1" appsapi "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog" ) @@ -61,6 +67,32 @@ func (c *Controller) Bootstrap() error { mgmtState = operatorapi.Removed } + infra, err := util.GetInfrastructure(c.listers) + if err != nil { + return err + } + + rolloutStrategy := appsapi.RollingUpdateDeploymentStrategyType + + // If Swift service is not available for OpenStack, we have to start using + // Cinder with RWO PVC backend. It means that we need to create an RWO claim + // and set the rollout strategy to Recreate. + switch infra.Status.PlatformStatus.Type { + case configapiv1.OpenStackPlatformType: + isSwiftEnabled, err := swift.IsSwiftEnabled(c.listers) + if err != nil { + return err + } + if !isSwiftEnabled { + err = c.createPVC(corev1.ReadWriteOnce) + if err != nil { + return err + } + + rolloutStrategy = appsapi.RecreateDeploymentStrategyType + } + } + cr = &imageregistryv1.Config{ ObjectMeta: metav1.ObjectMeta{ Name: defaults.ImageRegistryResourceName, @@ -73,7 +105,7 @@ func (c *Controller) Bootstrap() error { Storage: imageregistryv1.ImageRegistryConfigStorage{}, Replicas: 1, HTTPSecret: fmt.Sprintf("%x", string(secretBytes[:])), - RolloutStrategy: string(appsapi.RollingUpdateDeploymentStrategyType), + RolloutStrategy: string(rolloutStrategy), }, Status: imageregistryv1.ImageRegistryStatus{}, } @@ -94,3 +126,46 @@ func (c *Controller) Bootstrap() error { return nil } + +func (c *Controller) createPVC(accessMode corev1.PersistentVolumeAccessMode) error { + claimName := defaults.PVCImageRegistryName + + // Check that the claim does not exist before creating it + claim, err := c.clients.Core.PersistentVolumeClaims(defaults.ImageRegistryOperatorNamespace).Get(claimName, metav1.GetOptions{}) + if err != nil { + if !errors.IsNotFound(err) { + return err + } + + // "standard" is the default StorageClass name, that was provisioned by the cloud provider + storageClassName := "standard" + + claim = &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: claimName, + Namespace: defaults.ImageRegistryOperatorNamespace, + Annotations: map[string]string{ + pvc.PVCOwnerAnnotation: "true", + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + accessMode, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("100Gi"), + }, + }, + StorageClassName: &storageClassName, + }, + } + + _, err = c.clients.Core.PersistentVolumeClaims(defaults.ImageRegistryOperatorNamespace).Create(claim) + if err != nil { + return err + } + } + + return nil +} diff --git a/pkg/storage/pvc/pvc.go b/pkg/storage/pvc/pvc.go index 0be6b493e8..0d9519be86 100644 --- a/pkg/storage/pvc/pvc.go +++ b/pkg/storage/pvc/pvc.go @@ -24,7 +24,7 @@ import ( const ( rootDirectory = "/registry" randomSecretSize = 32 - pvcOwnerAnnotation = "imageregistry.openshift.io" + PVCOwnerAnnotation = "imageregistry.openshift.io" ) type driver struct { @@ -155,7 +155,7 @@ func (d *driver) createPVC(cr *imageregistryv1.Config) (*corev1.PersistentVolume Name: d.Config.Claim, Namespace: d.Namespace, Annotations: map[string]string{ - pvcOwnerAnnotation: "true", + PVCOwnerAnnotation: "true", }, }, Spec: corev1.PersistentVolumeClaimSpec{ @@ -181,7 +181,7 @@ func (d *driver) CreateStorage(cr *imageregistryv1.Config) error { ) if len(d.Config.Claim) == 0 { - d.Config.Claim = fmt.Sprintf("%s-storage", defaults.ImageRegistryName) + d.Config.Claim = defaults.PVCImageRegistryName // If there is no name and there is no PVC, then we will create a PVC. // If PVC is there and it was created by us, then just start using it again. @@ -236,7 +236,7 @@ func (d *driver) RemoveStorage(cr *imageregistryv1.Config) (retriable bool, err } func pvcIsCreatedByOperator(claim *corev1.PersistentVolumeClaim) (exist bool) { - _, exist = claim.Annotations[pvcOwnerAnnotation] + _, exist = claim.Annotations[PVCOwnerAnnotation] return } diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 6b10c413f4..0c481bf036 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -9,6 +9,7 @@ import ( configapiv1 "github.com/openshift/api/config/v1" imageregistryv1 "github.com/openshift/api/imageregistry/v1" + "github.com/openshift/cluster-image-registry-operator/defaults" regopclient "github.com/openshift/cluster-image-registry-operator/pkg/client" "github.com/openshift/cluster-image-registry-operator/pkg/envvar" "github.com/openshift/cluster-image-registry-operator/pkg/storage/azure" @@ -155,7 +156,17 @@ func GetPlatformStorage(listers *regopclient.Listers) (imageregistryv1.ImageRegi case configapiv1.GCPPlatformType: cfg.GCS = &imageregistryv1.ImageRegistryConfigStorageGCS{} case configapiv1.OpenStackPlatformType: - cfg.Swift = &imageregistryv1.ImageRegistryConfigStorageSwift{} + isSwiftEnabled, err := swift.IsSwiftEnabled(listers) + if err != nil { + return imageregistryv1.ImageRegistryConfigStorage{}, err + } + if !isSwiftEnabled { + cfg.PVC = &imageregistryv1.ImageRegistryConfigStoragePVC{ + Claim: defaults.PVCImageRegistryName, + } + } else { + cfg.Swift = &imageregistryv1.ImageRegistryConfigStorageSwift{} + } // Unknown platforms or LibVirt: we configure image registry using // EmptyDir storage.