Skip to content

Commit

Permalink
Remove emptyDir support
Browse files Browse the repository at this point in the history
closes: #1345
  • Loading branch information
git-hyagi committed Jan 2, 2025
1 parent 290be03 commit e3183a1
Show file tree
Hide file tree
Showing 16 changed files with 62 additions and 184 deletions.
1 change: 1 addition & 0 deletions CHANGES/1345.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Removed support for installations using `emptyDir`.
3 changes: 3 additions & 0 deletions config/samples/galaxy.azure.ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,6 @@ spec:
limits:
cpu: 800m
memory: 1Gi

database:
postgres_storage_class: standard
3 changes: 3 additions & 0 deletions config/samples/galaxy.s3.ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,6 @@ spec:
limits:
cpu: 800m
memory: 1Gi

database:
postgres_storage_class: standard
6 changes: 6 additions & 0 deletions config/samples/pulpproject_v1beta1_pulp_cr.galaxy.ocp.ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,9 @@ spec:
limits:
cpu: 800m
memory: 1Gi

file_storage_access_mode: "ReadWriteMany"
file_storage_size: "5Gi"
file_storage_storage_class: azurefile-csi
database:
postgres_storage_class: azurefile-csi
6 changes: 6 additions & 0 deletions config/samples/pulpproject_v1beta1_pulp_cr.ocp.ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,9 @@ spec:
limits:
cpu: 800m
memory: 1Gi

file_storage_access_mode: "ReadWriteMany"
file_storage_size: "5Gi"
file_storage_storage_class: azurefile-csi
database:
postgres_storage_class: azurefile-csi
1 change: 1 addition & 0 deletions config/samples/simple-external-cache.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ spec:

file_storage_access_mode: "ReadWriteOnce"
file_storage_size: "2Gi"
file_storage_storage_class: standard
signing_secret: "signing-galaxy"
signing_scripts: "signing-scripts"
image_web: "quay.io/pulp/galaxy-web"
Expand Down
28 changes: 0 additions & 28 deletions controllers/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,24 +496,6 @@ func (d *CommonDeployment) setVolumes(resources any, pulpcoreType settings.Pulpc
},
}
volumes = append(volumes, fileStorage)
} else if storageType[0] == EmptyDirType { // if there is no SC nor PVC nor object storage defined we will mount an emptyDir
emptyDir := corev1.Volume{
Name: "tmp-file-storage",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
}
volumes = append(volumes, emptyDir)
// only api pods need the assets-file-storage
if pulpcoreType == settings.API {
assetVolume := corev1.Volume{
Name: "assets-file-storage",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
}
volumes = append(volumes, assetVolume)
}
}

volumes = signingMetadataVolumes(resources, storageType, volumes)
Expand Down Expand Up @@ -663,13 +645,6 @@ func (d *CommonDeployment) setVolumeMounts(pulp repomanagerpulpprojectorgv1beta2
MountPath: "/var/lib/pulp",
}
volumeMounts = append(volumeMounts, fileStorageMount)
} else if storageType[0] == EmptyDirType { // if no file-storage nor object storage were provided we will mount the emptyDir
emptyDir := corev1.VolumeMount{Name: "tmp-file-storage", MountPath: "/var/lib/pulp/tmp"}
volumeMounts = append(volumeMounts, emptyDir)
if pulpcoreType == settings.API { // worker and content pods don't need to mount the assets-file-storage secret
assetsVolume := corev1.VolumeMount{Name: "assets-file-storage", MountPath: "/var/lib/pulp/assets"}
volumeMounts = append(volumeMounts, assetsVolume)
}
}

if pulp.Spec.SigningSecret != "" {
Expand Down Expand Up @@ -743,9 +718,6 @@ func (d *CommonDeployment) setInitContainerVolumeMounts(pulp repomanagerpulpproj
MountPath: "/var/lib/pulp",
}
volumeMounts = append(volumeMounts, fileStorageMount)
} else if storageType[0] == EmptyDirType { // if no file-storage nor object storage were provided we will mount the emptyDir
emptyDir := corev1.VolumeMount{Name: "tmp-file-storage", MountPath: "/var/lib/pulp/tmp"}
volumeMounts = append(volumeMounts, emptyDir)
}
d.initContainerVolumeMounts = append([]corev1.VolumeMount(nil), volumeMounts...)
}
Expand Down
68 changes: 3 additions & 65 deletions controllers/repo_manager/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package repo_manager_test
import (
"context"
"fmt"
"reflect"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -835,6 +834,8 @@ exec "${PULP_CONTENT_ENTRYPOINT[@]}" \
Raw: []byte(`{"Api_Root": "/pulp/"}`),
}

postgresStorageClass := "standard"

// [TODO] Instead of using this hardcoded pulp CR we should
// use the samples from config/samples/ folder during each
// pipeline workflow execution
Expand Down Expand Up @@ -868,6 +869,7 @@ exec "${PULP_CONTENT_ENTRYPOINT[@]}" \
Replicas: 1,
},
Database: repomanagerpulpprojectorgv1beta2.Database{
PostgresStorageClass: &postgresStorageClass,
PostgresStorageRequirements: "5Gi",
},
FileStorageAccessMode: "ReadWriteOnce",
Expand Down Expand Up @@ -973,70 +975,6 @@ exec "${PULP_CONTENT_ENTRYPOINT[@]}" \
})
})

Context("When pulp.Spec.Database.PostgresStorageClass and cluster SC are not defined", func() {
It("Should configure the database pod template with an emptyDir volume", func() {

By("Making sure that database type is not external")
if len(createdPulp.Spec.Database.ExternalDBSecret) > 0 {
Skip("External database does not need to provision a Persistent Volume")
}

By("Checking if postgressc is not defined")
if postgresSC := createdPulp.Spec.Database.PostgresStorageClass; postgresSC != nil && *postgresSC != "" {
Skip("PostgresSC defined")
}

By("Checking if there is no default SC")
if isDefaultSCDefined() {
Skip("Default storage class defined")
}

By("Checking if sts template is configured to use emptyDir volume")
var found bool
for _, volume := range createdSts.Spec.Template.Spec.Volumes {
if volume.Name == DBVolumeName && reflect.DeepEqual(volume.VolumeSource.EmptyDir, &corev1.EmptyDirVolumeSource{}) {
found = true
break
}
}
Expect(found).Should(BeTrue())
})
})

Context("When pulp is not configured with object storage nor pulp.Spec.FileStorageClass is defined and there is no default SC", func() {
It("Shoud configure the api pod template with an emptyDir volume", func() {
By("Checking if an object storage is not defined")
if len(createdPulp.Spec.ObjectStorageAzureSecret) != 0 || len(createdPulp.Spec.ObjectStorageS3Secret) != 0 {
Skip("Object storage defined")
}

By("Checking if fileSC is not defined")
if createdPulp.Spec.FileStorageClass != "" {
Skip("FileStorageClass defined")
}

By("Checking if there is no default SC")
if isDefaultSCDefined() {
Skip("Default storage class defined")
}

By("Checking if api deployment is configured to use emptyDir volume")
var foundTmp, foundAsset bool
apiDeployment := &appsv1.Deployment{}
objectGet(ctx, apiDeployment, PulpName+"-api")
for _, volume := range apiDeployment.Spec.Template.Spec.Volumes {
if volume.Name == "tmp-file-storage" && reflect.DeepEqual(volume.VolumeSource.EmptyDir, &corev1.EmptyDirVolumeSource{}) {
foundTmp = true
}
if volume.Name == "assets-file-storage" && reflect.DeepEqual(volume.VolumeSource.EmptyDir, &corev1.EmptyDirVolumeSource{}) {
foundAsset = true
}
}
Expect(foundTmp).Should(BeTrue())
Expect(foundAsset).Should(BeTrue())
})
})

Context("When creating API deployment", func() {
It("Should follow the spec from pulp CR", func() {
By("Checking api deployment being found")
Expand Down
12 changes: 0 additions & 12 deletions controllers/repo_manager/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ func (r *RepoManagerReconciler) databaseController(ctx context.Context, pulp *re
if err != nil && errors.IsNotFound(err) {
log.Info("Creating a new Database StatefulSet", "StatefulSet.Namespace", pgSts.Namespace, "StatefulSet.Name", statefulSetName)
controllers.UpdateStatus(ctx, r.Client, pulp, metav1.ConditionFalse, conditionType, "CreatingDatabaseSts", "Creating "+statefulSetName+" StatefulSet resource")
controllers.CheckEmptyDir(pulp, controllers.DatabaseResource)
// Set Pulp instance as the owner and controller
ctrl.SetControllerReference(pulp, expected_sts, r.Scheme)
err = r.Create(ctx, expected_sts)
Expand Down Expand Up @@ -337,17 +336,6 @@ func statefulSetForDatabase(m *repomanagerpulpprojectorgv1beta2.Pulp) *appsv1.St
}
volumes = append(volumes, volume)

// if there is no SC nor PVC nor object storage defined we will mount an emptyDir
} else if storageType[0] == controllers.EmptyDirType {
emptyDir := []corev1.Volume{
{
Name: volumeName,
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
},
}
volumes = append(volumes, emptyDir...)
}

pgDataMountPath := filepath.Dir(postgresDataPath)
Expand Down
40 changes: 29 additions & 11 deletions controllers/repo_manager/precheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func prechecks(ctx context.Context, r *RepoManagerReconciler, pulp *repomanagerp
}

// verify if multiple storage types were provided
if reconcile := checkMultipleStorageType(r.RawLogger, pulp); reconcile != nil {
if reconcile := checkStorageDefinitions(r.RawLogger, pulp); reconcile != nil {
return reconcile, nil
}

Expand All @@ -72,7 +72,7 @@ func prechecks(ctx context.Context, r *RepoManagerReconciler, pulp *repomanagerp
}

// verify inconsistency in file_storage_* definition
if reconcile := checkFileStorage(ctx, r, pulp); reconcile != nil {
if reconcile := checkFileStorage(r, pulp); reconcile != nil {
return reconcile, nil
}

Expand Down Expand Up @@ -155,13 +155,33 @@ func checkIngressDefinition(log logr.Logger, pulp *repomanagerpulpprojectorgv1be
return nil
}

// checkMultipleStorageType verifies if there is more than one storage type defined.
// checkStorageDefinitions verifies if there is more than one storage type defined or none.
// Only a single type should be provided, if more the operator will not be able to
// determine which one should be used.
func checkMultipleStorageType(log logr.Logger, pulp *repomanagerpulpprojectorgv1beta2.Pulp) *ctrl.Result {
func checkStorageDefinitions(log logr.Logger, pulp *repomanagerpulpprojectorgv1beta2.Pulp) *ctrl.Result {
for _, resource := range []string{controllers.PulpResource, controllers.CacheResource, controllers.DatabaseResource} {
if foundMultiStorage, storageType := controllers.MultiStorageConfigured(pulp, resource); foundMultiStorage {
log.Error(nil, "found more than one storage type \""+strings.Join(storageType, `", "`)+"\" for "+resource+". Please, choose only one storage type or do not define any to use emptyDir")
foundMultiStorage, storageType := controllers.MultiStorageConfigured(pulp, resource)
if foundMultiStorage {
log.Error(nil, "found more than one storage type \""+strings.Join(storageType, `", "`)+"\" for "+resource+". Please, choose only one storage type.")
return &ctrl.Result{}
}

// we don't need to check if there is no storage definition for cache pods (redis does not need to persist data)
// so we can skip the next checks and go to the next loop iteration
if resource == controllers.CacheResource {
continue
}

// we don't need to check if there is no storage definition for installations using an external postgres instance.
// so we can skip the next checks and go to the next loop iteration
if resource == controllers.DatabaseResource {
if pulp.Spec.Database.ExternalDBSecret != "" {
continue
}
}

if storageType == nil {
log.Error(nil, "could not find any storage definition for "+resource+". You must configure storage for Pulp and Database pods.")
return &ctrl.Result{}
}
}
Expand Down Expand Up @@ -215,18 +235,16 @@ func checkSecretsAvailability(ctx context.Context, r *RepoManagerReconciler, pul
// checkFileStorage verifies if there is a file_storage definition but the storage_class is not provided
// the file_storage_* fields are used to provision the PVC using the provided file_storage_class
// if no file_storage_class is provided, the other fields will not be useful and can cause confusion
func checkFileStorage(ctx context.Context, r *RepoManagerReconciler, pulp *repomanagerpulpprojectorgv1beta2.Pulp) *ctrl.Result {
func checkFileStorage(r *RepoManagerReconciler, pulp *repomanagerpulpprojectorgv1beta2.Pulp) *ctrl.Result {
if hasFileStorageDefinition(pulp) && len(pulp.Spec.FileStorageClass) == 0 {
r.RawLogger.Error(nil, "No file_storage_class provided for the file_storage_{access_mode,size} definition(s)!")
r.RawLogger.Error(nil, "Provide a file_storage_storage_class with the file_storage_{access_mode,size} fields to deploy Pulp with persistent data")
r.RawLogger.Error(nil, "or remove all file_storage_* fields to deploy Pulp with emptyDir.")
r.RawLogger.Error(nil, "Provide a file_storage_storage_class with the file_storage_{access_mode,size} fields to deploy Pulp with persistent data.")
return &ctrl.Result{}
}

if len(pulp.Spec.FileStorageClass) > 0 && (len(pulp.Spec.FileStorageAccessMode) == 0 || len(pulp.Spec.FileStorageSize) == 0) {
r.RawLogger.Error(nil, "file_storage_class provided but no file_storage_size and/or file_storage_access_mode defined!")
r.RawLogger.Error(nil, "Provide a file_storage_size and file_storage_access_mode fields to deploy Pulp with persistent data")
r.RawLogger.Error(nil, "or remove all file_storage_* fields to deploy Pulp with emptyDir.")
r.RawLogger.Error(nil, "Provide a file_storage_size and file_storage_access_mode fields to deploy Pulp with persistent data.")
return &ctrl.Result{}
}
return nil
Expand Down
6 changes: 0 additions & 6 deletions controllers/repo_manager/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,6 @@ func (r *RepoManagerReconciler) createPulpResource(resource ResourceDefinition,
log.Info("Creating a new "+resource.Name+" "+objKind, "Namespace", resource.Pulp.Namespace, "Name", resource.Name)
err = r.Create(resource.Context, expectedResource)

// special condition for api deployments where we need to provide a warning message
// in case no storage type is provided
if resource.Name == resource.Pulp.Name+"-api" && objKind == "Deployment" {
controllers.CheckEmptyDir(resource.Pulp, controllers.PulpResource)
}

if err != nil {
log.Error(err, "Failed to create new "+resource.Name+" "+objKind)
controllers.UpdateStatus(resource.Context, r.Client, resource.Pulp, metav1.ConditionFalse, resource.ConditionType, "ErrorCreating"+resource.Alias+objKind, "Failed to create "+resource.Name+" "+objKind+": "+err.Error())
Expand Down
15 changes: 2 additions & 13 deletions controllers/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func MultiStorageConfigured(pulp *repomanagerpulpprojectorgv1beta2.Pulp, resourc
if len(names) > 1 {
return true, names
} else if len(names) == 0 {
return false, []string{EmptyDirType}
return false, nil
}

case CacheResource:
Expand Down Expand Up @@ -179,7 +179,7 @@ func MultiStorageConfigured(pulp *repomanagerpulpprojectorgv1beta2.Pulp, resourc
if len(names) > 1 {
return true, names
} else if len(names) == 0 {
return false, []string{EmptyDirType}
return false, nil
}
}

Expand Down Expand Up @@ -282,17 +282,6 @@ func CustomZapLogger() *zap.Logger {
return logger
}

// CheckEmptyDir creates a log warn message in case no persistent storage is provided
// for the given resource
func CheckEmptyDir(pulp *repomanagerpulpprojectorgv1beta2.Pulp, resource string) {
_, storageType := MultiStorageConfigured(pulp, resource)
if storageType[0] == EmptyDirType {
logger := CustomZapLogger()
logger.Warn("No StorageClass or PVC defined for " + strings.ToUpper(resource) + " pods!")
logger.Warn("CONFIGURING " + strings.ToUpper(resource) + " POD VOLUME AS EMPTYDIR. THIS SHOULD NOT BE USED IN PRODUCTION CLUSTERS.")
}
}

// CheckImageVersionModified verifies if the container image tag defined in
// Pulp CR matches the one in the Deployment
func ImageChanged(pulp *repomanagerpulpprojectorgv1beta2.Pulp) bool {
Expand Down
4 changes: 2 additions & 2 deletions docs/configuring/database.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ Pulp operator provides a PostgreSQL database for Pulp to use, but it is also pos

[Pulp CR page](https://docs.pulpproject.org/pulp_operator/pulp/#database) has all the parameters that can be set to inform Pulp operator how it should deploy the PostgreSQL container.

If no `database` parameter is defined, Pulp operator will deploy PostgreSQL with the following configuration:
To configure Pulp operator to deploy PostgreSQL, **it is required to define the [storage configurations for the database pod](https://pulpproject.org/pulp-operator/docs/admin/guides/configurations/storage/#pulp-operator-storage-configuration)**.
Pulp operator will deploy PostgreSQL with the following configuration:

* a `StatefulSet` will be provisioned to handle PostgreSQL pod
* a single PostgreSQL replica will be available (it is **not** possible to form a cluster with this container)
* it will deploy a `docker.io/library/postgres:13` image
* **no data will be persisted**, the container will mount an emptyDir (all data will be lost in case of pod restart)


A new `Secret` (<deployment-name>-postgres-configuration) will also be created with some information like:
Expand Down
Loading

0 comments on commit e3183a1

Please sign in to comment.