diff --git a/pkg/apis/imageregistry/v1/types.go b/pkg/apis/imageregistry/v1/types.go index 57f57f04c1..531f7b7907 100644 --- a/pkg/apis/imageregistry/v1/types.go +++ b/pkg/apis/imageregistry/v1/types.go @@ -183,12 +183,17 @@ type ImageRegistryConfigStorageFilesystem struct { VolumeSource corev1.VolumeSource `json:"volumeSource,omitempty"` } +type ImageRegistryConfigStoragePVC struct { + Claim string `json:"claim,omitempty"` +} + type ImageRegistryConfigStorage struct { Azure *ImageRegistryConfigStorageAzure `json:"azure,omitempty"` Filesystem *ImageRegistryConfigStorageFilesystem `json:"filesystem,omitempty"` GCS *ImageRegistryConfigStorageGCS `json:"gcs,omitempty"` S3 *ImageRegistryConfigStorageS3 `json:"s3,omitempty"` Swift *ImageRegistryConfigStorageSwift `json:"swift,omitempty"` + PVC *ImageRegistryConfigStoragePVC `json:"pvc,omitempty"` } type ImageRegistryConfigRequests struct { diff --git a/pkg/apis/imageregistry/v1/zz_generated.deepcopy.go b/pkg/apis/imageregistry/v1/zz_generated.deepcopy.go index 2c45c6b320..c8dcd38bf2 100644 --- a/pkg/apis/imageregistry/v1/zz_generated.deepcopy.go +++ b/pkg/apis/imageregistry/v1/zz_generated.deepcopy.go @@ -164,6 +164,11 @@ func (in *ImageRegistryConfigStorage) DeepCopyInto(out *ImageRegistryConfigStora *out = new(ImageRegistryConfigStorageSwift) **out = **in } + if in.PVC != nil { + in, out := &in.PVC, &out.PVC + *out = new(ImageRegistryConfigStoragePVC) + **out = **in + } return } @@ -226,6 +231,22 @@ func (in *ImageRegistryConfigStorageGCS) DeepCopy() *ImageRegistryConfigStorageG return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ImageRegistryConfigStoragePVC) DeepCopyInto(out *ImageRegistryConfigStoragePVC) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageRegistryConfigStoragePVC. +func (in *ImageRegistryConfigStoragePVC) DeepCopy() *ImageRegistryConfigStoragePVC { + if in == nil { + return nil + } + out := new(ImageRegistryConfigStoragePVC) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ImageRegistryConfigStorageS3) DeepCopyInto(out *ImageRegistryConfigStorageS3) { *out = *in diff --git a/pkg/resource/caconfig.go b/pkg/resource/caconfig.go index 7ad79a8761..990338ed48 100644 --- a/pkg/resource/caconfig.go +++ b/pkg/resource/caconfig.go @@ -11,6 +11,7 @@ import ( configlisters "github.com/openshift/client-go/config/listers/config/v1" imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" "github.com/openshift/cluster-image-registry-operator/pkg/parameters" + "github.com/openshift/cluster-image-registry-operator/pkg/util" ) var _ Mutator = &generatorCAConfig{} @@ -35,7 +36,7 @@ func newGeneratorCAConfig(lister corelisters.ConfigMapNamespaceLister, imageConf imageConfigName: params.ImageConfig.Name, name: params.CAConfig.Name, namespace: params.Deployment.Namespace, - owner: asOwner(cr), + owner: util.AsOwner(cr), } } @@ -81,7 +82,7 @@ func (gcac *generatorCAConfig) expected() (runtime.Object, error) { } } - addOwnerRefToObject(cm, gcac.owner) + util.AddOwnerRefToObject(cm, gcac.owner) return cm, nil } diff --git a/pkg/resource/clusterrole.go b/pkg/resource/clusterrole.go index 940e461ff3..cf4f2ef3da 100644 --- a/pkg/resource/clusterrole.go +++ b/pkg/resource/clusterrole.go @@ -8,6 +8,7 @@ import ( rbaclisters "k8s.io/client-go/listers/rbac/v1" imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" + "github.com/openshift/cluster-image-registry-operator/pkg/util" ) var _ Mutator = &generatorClusterRole{} @@ -22,7 +23,7 @@ func newGeneratorClusterRole(lister rbaclisters.ClusterRoleLister, client rbacse return &generatorClusterRole{ lister: lister, client: client, - owner: asOwner(cr), + owner: util.AsOwner(cr), } } @@ -93,7 +94,7 @@ func (gcr *generatorClusterRole) expected() (runtime.Object, error) { }, } - addOwnerRefToObject(role, gcr.owner) + util.AddOwnerRefToObject(role, gcr.owner) return role, nil } diff --git a/pkg/resource/clusterrolebinding.go b/pkg/resource/clusterrolebinding.go index cec68d38cc..49c10a2b1f 100644 --- a/pkg/resource/clusterrolebinding.go +++ b/pkg/resource/clusterrolebinding.go @@ -9,6 +9,7 @@ import ( imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" "github.com/openshift/cluster-image-registry-operator/pkg/parameters" + "github.com/openshift/cluster-image-registry-operator/pkg/util" ) var _ Mutator = &generatorClusterRoleBinding{} @@ -27,7 +28,7 @@ func newGeneratorClusterRoleBinding(lister rbaclisters.ClusterRoleBindingLister, client: client, saName: params.Pod.ServiceAccount, saNamespace: params.Deployment.Namespace, - owner: asOwner(cr), + owner: util.AsOwner(cr), } } @@ -65,7 +66,7 @@ func (gcrb *generatorClusterRoleBinding) expected() (runtime.Object, error) { }, } - addOwnerRefToObject(crb, gcrb.owner) + util.AddOwnerRefToObject(crb, gcrb.owner) return crb, nil } diff --git a/pkg/resource/deployment.go b/pkg/resource/deployment.go index 2c54cd3a0b..27b4f76ffe 100644 --- a/pkg/resource/deployment.go +++ b/pkg/resource/deployment.go @@ -12,6 +12,7 @@ import ( imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" "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/util" ) var _ Mutator = &generatorDeployment{} @@ -83,7 +84,7 @@ func (gd *generatorDeployment) expected() (runtime.Object, error) { }, } - addOwnerRefToObject(deploy, asOwner(gd.cr)) + util.AddOwnerRefToObject(deploy, util.AsOwner(gd.cr)) return deploy, nil } diff --git a/pkg/resource/generator.go b/pkg/resource/generator.go index 647f12aa54..9efc9aadfd 100644 --- a/pkg/resource/generator.go +++ b/pkg/resource/generator.go @@ -171,6 +171,11 @@ func (g *Generator) removeObsoleteRoutes(cr *imageregistryv1.Config) error { } func (g *Generator) Apply(cr *imageregistryv1.Config) error { + err := g.syncStorage(cr) + if err != nil { + return fmt.Errorf("unable to sync storage configuration: %s", err) + } + generators, err := g.list(cr) if err != nil { return fmt.Errorf("unable to get generators: %s", err) @@ -214,11 +219,6 @@ func (g *Generator) Apply(cr *imageregistryv1.Config) error { return fmt.Errorf("unable to remove obsolete routes: %s", err) } - err = g.syncStorage(cr) - if err != nil { - return fmt.Errorf("unable to sync storage configuration: %s", err) - } - return nil } diff --git a/pkg/resource/imageconfig.go b/pkg/resource/imageconfig.go index e8d6abb30a..8b9db61401 100644 --- a/pkg/resource/imageconfig.go +++ b/pkg/resource/imageconfig.go @@ -19,6 +19,7 @@ import ( imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" "github.com/openshift/cluster-image-registry-operator/pkg/parameters" + "github.com/openshift/cluster-image-registry-operator/pkg/util" ) var _ Mutator = &generatorImageConfig{} @@ -43,7 +44,7 @@ func newGeneratorImageConfig(configLister configlisters.ImageLister, routeLister name: params.ImageConfig.Name, namespace: params.Deployment.Namespace, serviceName: params.Service.Name, - owner: asOwner(cr), + owner: util.AsOwner(cr), } } diff --git a/pkg/resource/route.go b/pkg/resource/route.go index 9e522fda09..ff8fbb2362 100644 --- a/pkg/resource/route.go +++ b/pkg/resource/route.go @@ -11,6 +11,7 @@ import ( imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" "github.com/openshift/cluster-image-registry-operator/pkg/parameters" + "github.com/openshift/cluster-image-registry-operator/pkg/util" ) var _ Mutator = &generatorRoute{} @@ -32,7 +33,7 @@ func newGeneratorRoute(lister routelisters.RouteNamespaceLister, secretLister co client: client, namespace: params.Deployment.Namespace, serviceName: params.Service.Name, - owner: asOwner(cr), + owner: util.AsOwner(cr), route: route, } } @@ -83,7 +84,7 @@ func (gr *generatorRoute) expected() (runtime.Object, error) { } } - addOwnerRefToObject(r, gr.owner) + util.AddOwnerRefToObject(r, gr.owner) return r, nil } diff --git a/pkg/resource/secret.go b/pkg/resource/secret.go index 97e9b368da..d687ecb4ba 100644 --- a/pkg/resource/secret.go +++ b/pkg/resource/secret.go @@ -10,6 +10,7 @@ import ( imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" "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/util" ) var _ Mutator = &generatorSecret{} @@ -30,7 +31,7 @@ func newGeneratorSecret(lister corelisters.SecretNamespaceLister, client coreset driver: driver, name: imageregistryv1.ImageRegistryPrivateConfiguration, namespace: params.Deployment.Namespace, - owner: asOwner(cr), + owner: util.AsOwner(cr), } } @@ -61,7 +62,7 @@ func (gs *generatorSecret) expected() (runtime.Object, error) { sec.StringData = data - addOwnerRefToObject(sec, gs.owner) + util.AddOwnerRefToObject(sec, gs.owner) return sec, nil } diff --git a/pkg/resource/service.go b/pkg/resource/service.go index c535cbe3e8..4e74d62326 100644 --- a/pkg/resource/service.go +++ b/pkg/resource/service.go @@ -13,6 +13,7 @@ import ( imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" "github.com/openshift/cluster-image-registry-operator/pkg/parameters" "github.com/openshift/cluster-image-registry-operator/pkg/resource/strategy" + "github.com/openshift/cluster-image-registry-operator/pkg/util" ) var _ Mutator = &generatorService{} @@ -37,7 +38,7 @@ func newGeneratorService(lister corelisters.ServiceNamespaceLister, client cores labels: params.Deployment.Labels, port: params.Container.Port, secretName: imageregistryv1.ImageRegistryName + "-tls", - owner: asOwner(cr), + owner: util.AsOwner(cr), } } @@ -77,7 +78,7 @@ func (gs *generatorService) expected() *corev1.Service { "service.alpha.openshift.io/serving-cert-secret-name": gs.secretName, } - addOwnerRefToObject(svc, gs.owner) + util.AddOwnerRefToObject(svc, gs.owner) return svc } diff --git a/pkg/resource/serviceaccount.go b/pkg/resource/serviceaccount.go index 8d9f35d52f..aad40308ef 100644 --- a/pkg/resource/serviceaccount.go +++ b/pkg/resource/serviceaccount.go @@ -9,6 +9,7 @@ import ( imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" "github.com/openshift/cluster-image-registry-operator/pkg/parameters" + "github.com/openshift/cluster-image-registry-operator/pkg/util" ) var _ Mutator = &generatorServiceAccount{} @@ -27,7 +28,7 @@ func newGeneratorServiceAccount(lister corelisters.ServiceAccountNamespaceLister client: client, name: params.Pod.ServiceAccount, namespace: params.Deployment.Namespace, - owner: asOwner(cr), + owner: util.AsOwner(cr), } } @@ -51,7 +52,7 @@ func (gsa *generatorServiceAccount) expected() (runtime.Object, error) { }, } - addOwnerRefToObject(sa, gsa.owner) + util.AddOwnerRefToObject(sa, gsa.owner) return sa, nil } diff --git a/pkg/resource/utils.go b/pkg/resource/utils.go deleted file mode 100644 index 5d7381a14e..0000000000 --- a/pkg/resource/utils.go +++ /dev/null @@ -1,26 +0,0 @@ -package resource - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" -) - -// addOwnerRefToObject appends the desired OwnerReference to the object -func addOwnerRefToObject(obj metav1.Object, ownerRef metav1.OwnerReference) { - obj.SetOwnerReferences(append(obj.GetOwnerReferences(), ownerRef)) -} - -// asOwner returns an OwnerReference set as the CR -func asOwner(cr *v1.Config) metav1.OwnerReference { - blockOwnerDeletion := true - isController := true - return metav1.OwnerReference{ - APIVersion: v1.SchemeGroupVersion.String(), - Kind: "Config", - Name: cr.GetName(), - UID: cr.GetUID(), - BlockOwnerDeletion: &blockOwnerDeletion, - Controller: &isController, - } -} diff --git a/pkg/storage/pvc/pvc.go b/pkg/storage/pvc/pvc.go new file mode 100644 index 0000000000..055d11fdc4 --- /dev/null +++ b/pkg/storage/pvc/pvc.go @@ -0,0 +1,223 @@ +package pvc + +import ( + "fmt" + "reflect" + "strings" + + 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" + coreset "k8s.io/client-go/kubernetes/typed/core/v1" + + operatorapi "github.com/openshift/api/operator/v1" + imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" + "github.com/openshift/cluster-image-registry-operator/pkg/storage/util" + operatorutil "github.com/openshift/cluster-image-registry-operator/pkg/util" + + regopclient "github.com/openshift/cluster-image-registry-operator/pkg/client" +) + +const ( + rootDirectory = "/registry" + randomSecretSize = 32 +) + +type driver struct { + Namespace string + Config *imageregistryv1.ImageRegistryConfigStoragePVC + Client *coreset.CoreV1Client +} + +func NewDriver(c *imageregistryv1.ImageRegistryConfigStoragePVC) (*driver, error) { + namespace, err := regopclient.GetWatchNamespace() + if err != nil { + return nil, fmt.Errorf("failed to get watch namespace: %s", err) + } + + kubeconfig, err := regopclient.GetConfig() + if err != nil { + return nil, err + } + + client, err := coreset.NewForConfig(kubeconfig) + if err != nil { + return nil, err + } + + return &driver{ + Namespace: namespace, + Config: c, + Client: client, + }, nil +} + +func (d *driver) ConfigEnv() (envs []corev1.EnvVar, err error) { + envs = append(envs, + corev1.EnvVar{Name: "REGISTRY_STORAGE", Value: "filesystem"}, + corev1.EnvVar{Name: "REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY", Value: rootDirectory}, + ) + return +} + +func (d *driver) Volumes() ([]corev1.Volume, []corev1.VolumeMount, error) { + vol := corev1.Volume{ + Name: "registry-storage", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: d.Config.Claim, + }, + }, + } + + mount := corev1.VolumeMount{ + Name: vol.Name, + MountPath: rootDirectory, + } + + return []corev1.Volume{vol}, []corev1.VolumeMount{mount}, nil +} + +func (d *driver) Secrets() (map[string]string, error) { + return nil, nil +} + +func (d *driver) StorageExists(cr *imageregistryv1.Config) (bool, error) { + if len(d.Config.Claim) != 0 { + _, err := d.Client.PersistentVolumeClaims(d.Namespace).Get(d.Config.Claim, metav1.GetOptions{}) + if err == nil { + util.UpdateCondition(cr, imageregistryv1.StorageExists, operatorapi.ConditionTrue, "PVC Exists", "") + return true, nil + } + if !errors.IsNotFound(err) { + util.UpdateCondition(cr, imageregistryv1.StorageExists, operatorapi.ConditionUnknown, fmt.Sprintf("Unknown error occurred checking for volume claim %s", d.Config.Claim), err.Error()) + return false, err + } + } + util.UpdateCondition(cr, imageregistryv1.StorageExists, operatorapi.ConditionFalse, "PVC does not exist", "") + return false, nil +} + +func (d *driver) StorageChanged(cr *imageregistryv1.Config) bool { + if !reflect.DeepEqual(cr.Status.Storage.PVC, cr.Spec.Storage.PVC) { + util.UpdateCondition(cr, imageregistryv1.StorageExists, operatorapi.ConditionUnknown, "PVC Configuration Changed", "PVC storage is in an old state") + return true + } + return false +} + +func (d *driver) checkPVC(cr *imageregistryv1.Config, claim *corev1.PersistentVolumeClaim) (err error) { + if claim == nil { + claim, err = d.Client.PersistentVolumeClaims(d.Namespace).Get(d.Config.Claim, metav1.GetOptions{}) + if err != nil { + return err + } + } + + requiredModes := []string{ + string(corev1.ReadWriteMany), + } + if cr.Spec.Replicas == 1 { + requiredModes = append(requiredModes, string(corev1.ReadWriteOnce)) + } + + for _, claimMode := range claim.Spec.AccessModes { + for _, mode := range requiredModes { + if string(claimMode) == mode { + return nil + } + } + } + + return fmt.Errorf("PVC %s does not contain the necessary access modes (%s)", d.Config.Claim, strings.Join(requiredModes, " or ")) +} + +func (d *driver) createPVC(cr *imageregistryv1.Config) (*corev1.PersistentVolumeClaim, error) { + claim := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: d.Config.Claim, + Namespace: d.Namespace, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteMany, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("100Gi"), + }, + }, + }, + } + + operatorutil.AddOwnerRefToObject(claim, operatorutil.AsOwner(cr)) + + return d.Client.PersistentVolumeClaims(d.Namespace).Create(claim) +} + +func (d *driver) CreateStorage(cr *imageregistryv1.Config) error { + var ( + err error + claim *corev1.PersistentVolumeClaim + storageManaged bool + ) + + if len(d.Config.Claim) == 0 { + d.Config.Claim = fmt.Sprintf("%s-storage", imageregistryv1.ImageRegistryName) + + // 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. + storageManaged = true + + claim, err = d.Client.PersistentVolumeClaims(d.Namespace).Get(d.Config.Claim, metav1.GetOptions{}) + if err == nil { + ownerRef := metav1.GetControllerOf(claim) + if ownerRef == nil || ownerRef.UID != operatorutil.AsOwner(cr).UID { + err = fmt.Errorf("could not create default PVC, it already exists and is not owned by the operator") + util.UpdateCondition(cr, imageregistryv1.StorageExists, operatorapi.ConditionFalse, "PVC Already Exists", err.Error()) + return err + } + } else if errors.IsNotFound(err) { + claim, err = d.createPVC(cr) + if err != nil { + util.UpdateCondition(cr, imageregistryv1.StorageExists, operatorapi.ConditionFalse, "Creation Failed", err.Error()) + return err + } + util.UpdateCondition(cr, imageregistryv1.StorageExists, operatorapi.ConditionTrue, "PVC Created", "") + } else { + return err + } + } else { + util.UpdateCondition(cr, imageregistryv1.StorageExists, operatorapi.ConditionTrue, "PVC Exists", "") + } + + if err := d.checkPVC(cr, claim); err != nil { + util.UpdateCondition(cr, imageregistryv1.StorageExists, operatorapi.ConditionFalse, "PVC Issues Found", err.Error()) + return err + } + + cr.Status.StorageManaged = storageManaged + cr.Status.Storage.PVC = d.Config.DeepCopy() + cr.Spec.Storage.PVC = d.Config.DeepCopy() + + return nil +} + +func (d *driver) RemoveStorage(cr *imageregistryv1.Config) (retriable bool, err error) { + if !cr.Status.StorageManaged || len(d.Config.Claim) == 0 { + return false, nil + } + + err = d.Client.PersistentVolumeClaims(d.Namespace).Delete(d.Config.Claim, &metav1.DeleteOptions{}) + if err != nil && !errors.IsNotFound(err) { + return false, err + } + + return false, nil +} + +func (d *driver) CompleteConfiguration(cr *imageregistryv1.Config) error { + cr.Status.Storage.PVC = d.Config.DeepCopy() + return nil +} diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 3931efda29..046da53750 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -12,6 +12,7 @@ import ( "github.com/openshift/cluster-image-registry-operator/pkg/storage/emptydir" "github.com/openshift/cluster-image-registry-operator/pkg/storage/filesystem" "github.com/openshift/cluster-image-registry-operator/pkg/storage/gcs" + "github.com/openshift/cluster-image-registry-operator/pkg/storage/pvc" "github.com/openshift/cluster-image-registry-operator/pkg/storage/s3" "github.com/openshift/cluster-image-registry-operator/pkg/storage/swift" ) @@ -65,6 +66,15 @@ func newDriver(cfg *imageregistryv1.ImageRegistryConfigStorage, listers *regopcl drivers = append(drivers, swift.NewDriver(cfg.Swift, listers)) } + if cfg.PVC != nil { + drv, err := pvc.NewDriver(cfg.PVC) + if err != nil { + return nil, err + } + names = append(names, "PVC") + drivers = append(drivers, drv) + } + switch len(drivers) { case 0: return nil, ErrStorageNotConfigured @@ -117,10 +127,7 @@ func getPlatformStorage() (imageregistryv1.ImageRegistryConfigStorage, error) { }, } default: - // if we can't determine what platform we're on, fallback to creating - // a PVC for the registry. - // TODO: implement this idea - return cfg, nil + cfg.PVC = &imageregistryv1.ImageRegistryConfigStoragePVC{} } return cfg, nil diff --git a/pkg/util/util.go b/pkg/util/util.go index 73f1cd3865..a31354a8a9 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -4,6 +4,8 @@ import ( "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" ) func ObjectInfo(o interface{}) string { @@ -15,3 +17,22 @@ func ObjectInfo(o interface{}) string { s += fmt.Sprintf("Name=%s", object.GetName()) return s } + +// AddOwnerRefToObject appends the desired OwnerReference to the object +func AddOwnerRefToObject(obj metav1.Object, ownerRef metav1.OwnerReference) { + obj.SetOwnerReferences(append(obj.GetOwnerReferences(), ownerRef)) +} + +// AsOwner returns an OwnerReference set as the CR +func AsOwner(cr *v1.Config) metav1.OwnerReference { + blockOwnerDeletion := true + isController := true + return metav1.OwnerReference{ + APIVersion: v1.SchemeGroupVersion.String(), + Kind: "Config", + Name: cr.GetName(), + UID: cr.GetUID(), + BlockOwnerDeletion: &blockOwnerDeletion, + Controller: &isController, + } +} diff --git a/test/e2e/configuration_test.go b/test/e2e/configuration_test.go index c51b15ed82..a6ca094ce4 100644 --- a/test/e2e/configuration_test.go +++ b/test/e2e/configuration_test.go @@ -39,7 +39,7 @@ func TestPodResourceConfiguration(t *testing.T) { Replicas: 1, Resources: &corev1.ResourceRequirements{ Limits: corev1.ResourceList{ - "memory": resource.MustParse("512Mi"), + corev1.ResourceMemory: resource.MustParse("512Mi"), }, }, NodeSelector: map[string]string{ @@ -61,9 +61,10 @@ func TestPodResourceConfiguration(t *testing.T) { for _, deployment := range deployments.Items { if strings.HasPrefix(deployment.Name, "image-registry") { - mem, ok := deployment.Spec.Template.Spec.Containers[0].Resources.Limits["memory"] + mem, ok := deployment.Spec.Template.Spec.Containers[0].Resources.Limits[corev1.ResourceMemory] if !ok { - t.Errorf("no memory limit set on registry deployment: %#v", deployment) + framework.DumpYAML(t, "deployment", deployment) + t.Errorf("no memory limit set on registry deployment") } if mem.String() != "512Mi" { t.Errorf("expected memory limit of 512Mi, found: %s", mem.String()) diff --git a/test/e2e/pvc_test.go b/test/e2e/pvc_test.go new file mode 100644 index 0000000000..db3e9e8919 --- /dev/null +++ b/test/e2e/pvc_test.go @@ -0,0 +1,247 @@ +package e2e_test + +import ( + "fmt" + "testing" + "time" + + 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/apimachinery/pkg/util/rand" + "k8s.io/apimachinery/pkg/util/wait" + + operatorapi "github.com/openshift/api/operator/v1" + + imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1" + "github.com/openshift/cluster-image-registry-operator/test/framework" +) + +func testDefer(t *testing.T, client *framework.Clientset) { + if t.Failed() { + scList, err := client.StorageClasses().List(metav1.ListOptions{}) + if err != nil { + t.Logf("unable to dump the storage classes: %s", err) + } else { + framework.DumpYAML(t, "storageclasses", scList) + } + + pvList, err := client.PersistentVolumes().List(metav1.ListOptions{}) + if err != nil { + t.Logf("unable to dump the persistent volumes: %s", err) + } else { + framework.DumpYAML(t, "persistentvolumes", pvList) + } + + pvcList, err := client.PersistentVolumeClaims(imageregistryv1.ImageRegistryOperatorNamespace).List(metav1.ListOptions{}) + if err != nil { + t.Logf("unable to dump the persistent volume claims: %s", err) + } else { + framework.DumpYAML(t, "persistentvolumeclaims", pvcList) + } + } + framework.MustRemoveImageRegistry(t, client) +} + +func createPV(t *testing.T, storageClass string) error { + client := framework.MustNewClientset(t, nil) + name := "" + + for i := 0; i < 100; i++ { + pvName := fmt.Sprintf("pv-%s", rand.String(64)) + localPath := fmt.Sprintf("/tmp/%s", pvName) + + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvName, + }, + Spec: corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("100Gi"), + }, + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + corev1.ReadOnlyMany, + corev1.ReadWriteMany, + }, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: localPath, + }, + }, + StorageClassName: storageClass, + }, + } + + _, err := client.PersistentVolumes().Create(pv) + if err == nil { + t.Logf("PersistentVolume %s created", pvName) + name = pvName + break + } + + if errors.IsAlreadyExists(err) { + continue + } + + t.Logf("unable to create PersistentVolume %s: %s", pvName, err) + } + + if name == "" { + return fmt.Errorf("unable to create PersistentVolume") + } + + err := wait.Poll(1*time.Second, framework.AsyncOperationTimeout, func() (bool, error) { + pv, pvErr := client.PersistentVolumes().Get(name, metav1.GetOptions{}) + return (pv != nil && pv.Status.Phase != corev1.VolumePending), pvErr + }) + + if err != nil { + return err + } + + return nil +} + +func createPVWithStorageClass(t *testing.T) error { + client := framework.MustNewClientset(t, nil) + + storageClassList, err := client.StorageClasses().List(metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("unable to list storage classes: %s", err) + } + + for _, storageClass := range storageClassList.Items { + if err := createPV(t, storageClass.Name); err != nil { + return err + } + } + + return nil +} + +func createPVC(t *testing.T, name string) error { + client := framework.MustNewClientset(t, nil) + + claim := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: imageregistryv1.ImageRegistryOperatorNamespace, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteMany, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }, + }, + }, + } + + _, err := client.PersistentVolumeClaims(imageregistryv1.ImageRegistryOperatorNamespace).Create(claim) + if err != nil { + return err + } + + err = wait.Poll(1*time.Second, framework.AsyncOperationTimeout, func() (bool, error) { + pvc, pvcErr := client.PersistentVolumeClaims(imageregistryv1.ImageRegistryOperatorNamespace).Get(name, metav1.GetOptions{}) + return (pvc != nil && pvc.Status.Phase != corev1.ClaimPending), pvcErr + }) + + if err != nil { + return err + } + + return nil +} + +func checkTestResult(t *testing.T, client *framework.Clientset) { + framework.MustEnsureImageRegistryIsAvailable(t, client) + framework.MustEnsureInternalRegistryHostnameIsSet(t, client) + framework.MustEnsureClusterOperatorStatusIsSet(t, client) + framework.MustEnsureOperatorIsNotHotLooping(t, client) + + deploy, err := client.Deployments(imageregistryv1.ImageRegistryOperatorNamespace).Get(imageregistryv1.ImageRegistryName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + if deploy.Status.AvailableReplicas == 0 { + framework.DumpYAML(t, "deployment", deploy) + t.Errorf("error: the deployment doesn't have available replicas") + } +} + +func TestDefaultPVC(t *testing.T) { + client := framework.MustNewClientset(t, nil) + + defer testDefer(t, client) + + if err := createPV(t, ""); err != nil { + t.Fatal(err) + } + + if err := createPVWithStorageClass(t); err != nil { + t.Fatal(err) + } + + framework.MustDeployImageRegistry(t, client, &imageregistryv1.Config{ + TypeMeta: metav1.TypeMeta{ + APIVersion: imageregistryv1.SchemeGroupVersion.String(), + Kind: "Config", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: imageregistryv1.ImageRegistryResourceName, + }, + Spec: imageregistryv1.ImageRegistrySpec{ + ManagementState: operatorapi.Managed, + Storage: imageregistryv1.ImageRegistryConfigStorage{ + PVC: &imageregistryv1.ImageRegistryConfigStoragePVC{}, + }, + Replicas: 1, + }, + }) + + checkTestResult(t, client) +} + +func TestCustomPVC(t *testing.T) { + client := framework.MustNewClientset(t, nil) + + defer testDefer(t, client) + + if err := createPV(t, ""); err != nil { + t.Fatal(err) + } + + if err := createPVWithStorageClass(t); err != nil { + t.Fatal(err) + } + + if err := createPVC(t, "test-custom-pvc"); err != nil { + t.Fatal(err) + } + + framework.MustDeployImageRegistry(t, client, &imageregistryv1.Config{ + TypeMeta: metav1.TypeMeta{ + APIVersion: imageregistryv1.SchemeGroupVersion.String(), + Kind: "Config", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: imageregistryv1.ImageRegistryResourceName, + }, + Spec: imageregistryv1.ImageRegistrySpec{ + ManagementState: operatorapi.Managed, + Storage: imageregistryv1.ImageRegistryConfigStorage{ + PVC: &imageregistryv1.ImageRegistryConfigStoragePVC{ + Claim: "test-custom-pvc", + }, + }, + Replicas: 1, + }, + }) + + checkTestResult(t, client) +} diff --git a/test/framework/clientset.go b/test/framework/clientset.go index 2321c2f7ac..79a5bc1ccb 100644 --- a/test/framework/clientset.go +++ b/test/framework/clientset.go @@ -6,6 +6,7 @@ import ( clientappsv1 "k8s.io/client-go/kubernetes/typed/apps/v1" clientcorev1 "k8s.io/client-go/kubernetes/typed/core/v1" + clientstoragev1 "k8s.io/client-go/kubernetes/typed/storage/v1" restclient "k8s.io/client-go/rest" clientconfigv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" @@ -21,6 +22,7 @@ type Clientset struct { clientconfigv1.ConfigV1Interface clientimageregistryv1.ImageregistryV1Interface clientroutev1.RouteV1Interface + clientstoragev1.StorageV1Interface } // NewClientset creates a set of Kubernetes clients. The default kubeconfig is @@ -54,6 +56,10 @@ func NewClientset(kubeconfig *restclient.Config) (clientset *Clientset, err erro if err != nil { return } + clientset.StorageV1Interface, err = clientstoragev1.NewForConfig(kubeconfig) + if err != nil { + return + } return } diff --git a/test/framework/framework.go b/test/framework/framework.go index d661b537c1..2e5d39ef55 100644 --- a/test/framework/framework.go +++ b/test/framework/framework.go @@ -1,10 +1,12 @@ package framework import ( + "fmt" "testing" "time" "github.com/davecgh/go-spew/spew" + "github.com/ghodss/yaml" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -33,6 +35,15 @@ func DumpObject(logger Logger, prefix string, obj interface{}) { logger.Logf("%s:\n%s", prefix, spew.Sdump(obj)) } +// DumpYAML prints the object to the test log as YAML. +func DumpYAML(logger Logger, prefix string, obj interface{}) { + data, err := yaml.Marshal(obj) + if err != nil { + panic(fmt.Sprintf("failed to marshal object: %s", err)) + } + logger.Logf("%s:\n%s", prefix, string(data)) +} + // DeleteCompletely sends a delete request and waits until the resource and // its dependents are deleted. func DeleteCompletely(getObject func() (metav1.Object, error), deleteObject func(*metav1.DeleteOptions) error) error { diff --git a/test/framework/imageregistry.go b/test/framework/imageregistry.go index 7de3465d1a..837b03b601 100644 --- a/test/framework/imageregistry.go +++ b/test/framework/imageregistry.go @@ -100,7 +100,7 @@ func ensureImageRegistryToBeRemoved(logger Logger, client *Clientset) error { return conds.Progressing.IsFalse() && conds.Removed.IsTrue(), nil }) if err != nil { - DumpObject(logger, "the latest observed state of the image registry resource", cr) + DumpYAML(logger, "the latest observed state of the image registry resource", cr) DumpOperatorLogs(logger, client) return fmt.Errorf("failed to wait for the imageregistry resource to be removed: %s", err) } @@ -185,7 +185,7 @@ func DumpImageRegistryResource(logger Logger, client *Clientset) { logger.Logf("unable to dump the image registry resource: %s", err) return } - DumpObject(logger, "the image registry resource", cr) + DumpYAML(logger, "the image registry resource", cr) } func ensureImageRegistryIsProcessed(logger Logger, client *Clientset) (*imageregistryapiv1.Config, error) { @@ -205,7 +205,7 @@ func ensureImageRegistryIsProcessed(logger Logger, client *Clientset) (*imagereg return conds.Progressing.IsFalse() && conds.Available.IsTrue() || conds.Failing.IsTrue(), nil }) if err != nil { - DumpObject(logger, "the latest observed state of the image registry resource", cr) + DumpYAML(logger, "the latest observed state of the image registry resource", cr) DumpOperatorLogs(logger, client) return cr, fmt.Errorf("failed to wait for the imageregistry resource to be processed: %s", err) } @@ -230,7 +230,7 @@ func ensureImageRegistryIsAvailable(logger Logger, client *Clientset) error { conds := GetImageRegistryConditions(cr) if conds.Progressing.IsTrue() || conds.Available.IsFalse() { - DumpObject(logger, "the latest observed state of the image registry resource", cr) + DumpYAML(logger, "the latest observed state of the image registry resource", cr) DumpOperatorLogs(logger, client) return fmt.Errorf("the imageregistry resource is processed, but the the image registry is not available") }