Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move GlanceAPI deployment to Statefulset #352

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

fmount
Copy link
Contributor

@fmount fmount commented Oct 26, 2023

This patch improves the way the glance statefulset uses PVCs:

  • ensurePVC is not required anymore and the logic has been moved away from controller to the statefulset creation function
  • storageClass and storageRequest are now fields of the GlanceAPITemplate
  • getVolumes don't require pvc(s) anymore
  • DBSync only mounts what it needs (Do not require DbSync to mount all the Glance volumes #351)
  • GlanceAPI Pvc is moved from RWX to RWO

@fmount
Copy link
Contributor Author

fmount commented Oct 27, 2023

/retest

Signed-off-by: Francesco Pantano <[email protected]>
This patch improves the way the glance statefulset uses PVCs:

- ensurePVC is not required anymore and the logic has been moved away
  from controller to the statefulset creation function
- storage{class, Request} are now fields of the GlanceAPITemplate
- getVolumes don't require the pvc anymore

Signed-off-by: Francesco Pantano <[email protected]>
@fmount fmount force-pushed the statefulset branch 3 times, most recently from fda8e38 to 332e407 Compare October 30, 2023 13:26
@fmount fmount changed the title WIP - DNM - Statefulset Move GlanceAPI deployment to Statefulset Oct 30, 2023
fultonj added a commit to fultonj/architecture that referenced this pull request Oct 31, 2023
We will restore this after the following merges.

openstack-k8s-operators/glance-operator#352

Signed-off-by: John Fulton <[email protected]>
fultonj added a commit to fultonj/architecture that referenced this pull request Oct 31, 2023
In the current implemenation this is causing pods to
get stuck in pending. Remove until we can implement
correctly after the following merges.

openstack-k8s-operators/glance-operator#352

Signed-off-by: John Fulton <[email protected]>
@fmount
Copy link
Contributor Author

fmount commented Nov 2, 2023

@mdbooth and I tested it in a live (baremetal) environement and it works as expected.
/cc @fultonj @abays

@@ -167,12 +167,6 @@ type GlanceStatus struct {

// Glance Database Hostname
DatabaseHostname string `json:"databaseHostname,omitempty"`

// ReadyCount of internal and admin Glance API instance
GlanceAPIInternalReadyCount int32 `json:"glanceAPIInternalReadyCount,omitempty"`
Copy link
Contributor Author

@fmount fmount Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abays this is not required anymore w/ statefulset (same story with the other parameter)

@@ -4,7 +4,7 @@ metadata:
name: image-import-staging-workspace
spec:
accessModes:
- ReadWriteMany
- ReadWriteOnce
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each glance pod is going to have stable access to is own pvc

glance-0 -> pvc-0
glance-1 -> pvc-1
glance-2 -> pvc-2

@@ -284,15 +283,6 @@ func (r *GlanceReconciler) reconcileInit(
) (ctrl.Result, error) {
r.Log.Info(fmt.Sprintf("Reconciling Service '%s' init", instance.Name))

// Define the PVCs objects required by Glance
ctrlResult, err := r.ensurePVC(ctx, helper, instance, serviceLabels)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moving to statefulset in the first place allow us to remove the PV creation logic from the glance-controller

@@ -699,6 +687,10 @@ func (r *GlanceReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst
apiSpec.GlanceAPITemplate.NodeSelector = instance.Spec.NodeSelector
}

// Inherit the values required for PVC creation from the top-level CR
apiSpec.GlanceAPITemplate.StorageRequest = instance.Spec.StorageRequest
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the instances needs to be tied to the same StorageClass and the same StorageRequest, hence this propagates the two fields to the statefulset that is supposed to create them via VolumeClaimTemplates

Copy link
Contributor

@abays abays left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Nov 2, 2023
Copy link
Contributor

openshift-ci bot commented Nov 2, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abays, fmount

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot merged commit 956e8e4 into openstack-k8s-operators:main Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants