Object Storage (LEP 20230430) #2136

m-ildefons · 2023-09-06T08:54:25Z

Implementation of an ObjectStore CRD and an object storage controller.
This controller watches for ObjectStore resources on the K8s API and orchestrates instances of the s3gw and its UI on top of a Longhorn volume. This allows an operator to configure object storage on a Longhorn installation.

Related: longhorn/longhorn#6640
Related: longhorn/longhorn-ui#649

LEP: longhorn/longhorn#5832

mergify · 2023-09-06T08:55:04Z

This pull request is now in conflicts. Could you fix it @m-ildefons? 🙏

types/object_store.go

k8s/pkg/apis/longhorn/v1beta2/objectstore.go

mergify · 2023-09-12T15:41:34Z

This pull request is now in conflicts. Could you fix it @m-ildefons? 🙏

mergify · 2023-10-06T03:20:53Z

This pull request is now in conflicts. Could you fix it @m-ildefons? 🙏

k8s/pkg/apis/longhorn/v1beta2/objectstore.go

controller/object_store_controller.go

k8s/pkg/apis/longhorn/v1beta2/objectstore.go

controller/object_store_controller.go

k8s/pkg/apis/longhorn/v1beta2/objectstore.go

controller/object_store_controller.go

datastore/longhorn.go

controller/object_store_controller.go

Fix review remarks Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Remove ingres for the object store UI. This is no longer needed when using the Longhorn UI's nginx as reverse proxy. Fix deleting an object store by removing the finalizer without waiting on resources to be deleted. Since OwnerReferences are used for automatic cleanup, the finalizer can be removed immediately. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Fix websocket controller: Relay secret, object store and settings information as expected by the UI frontend and as used by the object store tab in the UI Error handling: Remove custom error variables from the object store controller, since in Go, errors created with the errors.New function can not be compared with the errors.Is function as expected This mechanism is replaced by just wrapping normal errors with errors.Wrapf Error handling: Fix some conditions in checkDeployment. When a deployment is scaled up, there is a brief period where it has 0 replicas and 0 unavailable replicas. Therefore it is insufficient to just check the count of unavailable replicas to make sure the deployment is ready, the total count of replicas has to checked too. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Expose size and usage information of the longhorn volume associated with an object store via the internal API to the UI. This allows the Longhorn UI to receive information about the actual size and free space in an object store. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Fix event trigger for PVC events. This had misakenly been wired up to call the event handler for Service events, but now it's fixed to call the right event handler. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Fix creating a secret when given credentials from the UI. Instead of over-writing an existing secret or similar antics, generate a name without a conflict for a new secret to use. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Fix volume expansion: - ensure a volume expansion is only tried when the new size is larger than the old size - propagate the new size to the Longhorn volume, thereby expanding it, when an object store is updated Fix update: - Propagate the container image from the UI to the deployment, thereby allowing users to update the s3gw container images from the Longhorn UI even for existing object stores and even to newer versions than those that are originally shipped Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

- Fix s3gw status page container port - Force setting the `.spec.image` and `.spec.uiImage` properties of the object store via webhook on creation - Don't transition the object store to error state when creating resources fails, instead retry - Check PV and Longhorn Volume health - Don't check service health as a K8s Service resource doesn't have any status indicating healthyness or not. It either exists or not, readiness is dealt with on the deployment/pod level Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

- Update CRDs, move size property under .spec, rename .spec.storage to .spec.volumeParameters - Improve webhook to make storage attributes immutable if they don't also change the backing volume - Improve error handling in the controller Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Update CRDs Safe image modifications when updating images Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Fix secret creation with logic that supports reclaiming managed secrets and errors out when user-created secrets are conflicting with to-be-created ones Adapt the kubernetes config map controller to be able to create and manage multiple storage classes. Add default settings to allow the kubernetes config map controller to maanger the stub storage class for the volumes of object stores Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Stop propagating image settings to the object store controller. Applying default image settings to object stores is handled in the webhooks. Therefore the object store controller no longer needs access to this information, besides it would have to be fetched from the appropriate setting anyways. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Fix the race condition on start of a new object store. As long as there is just the object store and no longhorn volume yet some controller needs to be responsible for the object store. Usually it would be the one responsible for the volume, this doesn't exist yet and there are no provisions to track the owner another way. To fix this, just let the controller on the first node handle it. Fix error handling by setting the object store state to error only once and otherwise propagate errors up to the reconcile function. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

- Adjust container launch options for the s3gw to match the new launch options for s3gw v0.23.0 and up - Cleanup access credential secrets when they have been created through the longhorn manager and the object store is deleted Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

- Fix potential nil pointer dereference when receiving API errors other than ErrNotFound - Export Object Store size specs in the prometheus exporter - Send the number of deployed object stores to the telemetry endpoint (if sending telemetry is enabled) longhorn/longhorn#6720 Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

- Add admission webhook for validating object store resources - Force foreground cascading deleteion of object stores. This ensures that the volume and PV are cleaned up by the object store controller and resources aren't leaked before removing the finalizer on the object store. - Consolidate starting state. The object store's state is only set to starting in the `initializeObjectStore` function, not whenever resources are created - Error conditions: All errors except server-side API errors will cause the object store state to become `error`. There is only one place in the code where the state can be set to `error`, which is at the end of the reconcile function. - Telemetry setting: Depending on the Longhorn setting allowing for sending telemetry, the telemetry for the s3gw is switchen on or off. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

- Reorder the health checks during the creation of an object store When creating an object store, the responsibility for managing the resources making up that object store rests with the controller that's also responsible for the longhorn volume. There is a brief moment while the object store is starting when the information which controller is responsible for the longhorn volume has not yet been propagated to all controllers. Therefore the controller must first wait until the longhorn volume is healthy and has an owner before it can determine for sure if it's responsible or not. Only then the other resources can be created without running into race conditions with other controller who think they are responsible. - Clean up utility functions and constants Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

Rebase on master and update vendored modules Fix logic in test script. To ensure only the necessary unit tests are executed (or of nothing has been changed, all unit tests are executed), the logic needs to make sure not to interprete vendored modules as changed source since it will fail to find unit tests for them. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

m-ildefons · 2023-11-29T10:05:11Z

In the future upgrade, if we want to change the default manifests for pv/pvc/dpl/endpoints/svc/secrets, what will we do?

There is currently nothing planned for that case, since it's unclear to me what situations exactly can occur and I don't expect the manifests to change a lot.
We can easily change the manifests that are hard-coded in the controller to match the corresponding version of the s3gw. This works for all newly deployed object stores. But if you're worried about upgrading existing object stores, the story gets much more complicated. We would essentially need versioned manifests.

Can you describe what situation exactly you're thinking about here?

PhanLe1010 · 2023-11-29T19:45:41Z

controller/object_store_controller.go

+	// remove the object store resource as well.
+	_, err = osc.ds.GetService(osc.namespace, store.Name)
+	if err == nil {
+		return osc.ds.DeleteService(osc.namespace, store.Name)


This means that if the osc.ds.DeleteService() returns nil, we will return nil and never requeue the ObjectStore object again until 1 hour resync period hit. Could you take a look at the suggesting modification change below

PhanLe1010 · 2023-11-29T19:45:45Z

controller/object_store_controller.go

+func (osc *ObjectStoreController) handleTerminating(store *longhorn.ObjectStore) (err error) {
+	// The resources are created in the order:
+	// Volume -> PV -> PVC -> Deployment -> Service
+	// so we tear them down in reverse:
+	// Service -> Deployment -> PVC -> PV -> Volume
+	// Once that is done we can remove the finalizer, which allows the K8s API to
+	// remove the object store resource as well.
+	_, err = osc.ds.GetService(osc.namespace, store.Name)
+	if err == nil {
+		return osc.ds.DeleteService(osc.namespace, store.Name)
+	} else if !datastore.ErrorIsNotFound(err) {
+		return err
+	}
+
+	_, err = osc.ds.GetDeployment(store.Name)
+	if err == nil {
+		return osc.ds.DeleteDeployment(store.Name)
+	} else if !datastore.ErrorIsNotFound(err) {
+		return err
+	}
+
+	_, err = osc.ds.GetPersistentVolumeClaim(osc.namespace, store.Name)
+	if err == nil {
+		return osc.ds.DeletePersistentVolumeClaim(osc.namespace, store.Name)
+	} else if !datastore.ErrorIsNotFound(err) {
+		return err
+	}
+
+	_, err = osc.ds.GetPersistentVolume(store.Name)
+	if err == nil {
+		osc.ds.DeletePersistentVolume(store.Name)
+	} else if !datastore.ErrorIsNotFound(err) {
+		return err
+	}
+
+	_, err = osc.ds.GetVolume(store.Name)
+	if err == nil {
+		osc.ds.DeleteVolume(store.Name)
+	} else if !datastore.ErrorIsNotFound(err) {
+		return err
+	}
+
+	// cleanup all secrets with matching labels.
+	labels := types.GetBaseLabelsForSystemManagedComponent()
+	labels[types.GetLonghornLabelComponentKey()] = types.LonghornLabelObjectStore
+	labels[types.GetLonghornLabelKey(types.LonghornLabelObjectStore)] = store.Name
+
+	secrets, err := osc.ds.ListSecretsByLabels(osc.namespace, labels)
+	if err != nil {
+		return err
+	}
+
+	for _, secret := range secrets {
+		osc.ds.DeleteSecret(osc.namespace, secret.Name)
+	}
+
+	if len(store.ObjectMeta.Finalizers) != 0 {
+		return osc.ds.RemoveFinalizerForObjectStore(store)
+	}
+
+	return nil
+}


suggesting to directly delete the object and check the returned error instead of getting the object then delete

Suggested change

func (osc *ObjectStoreController) handleTerminating(store *longhorn.ObjectStore) (err error) {

// The resources are created in the order:

// Volume -> PV -> PVC -> Deployment -> Service

// so we tear them down in reverse:

// Service -> Deployment -> PVC -> PV -> Volume

// Once that is done we can remove the finalizer, which allows the K8s API to

// remove the object store resource as well.

_, err = osc.ds.GetService(osc.namespace, store.Name)

if err == nil {

return osc.ds.DeleteService(osc.namespace, store.Name)

} else if !datastore.ErrorIsNotFound(err) {

return err

}

_, err = osc.ds.GetDeployment(store.Name)

if err == nil {

return osc.ds.DeleteDeployment(store.Name)

} else if !datastore.ErrorIsNotFound(err) {

return err

}

_, err = osc.ds.GetPersistentVolumeClaim(osc.namespace, store.Name)

if err == nil {

return osc.ds.DeletePersistentVolumeClaim(osc.namespace, store.Name)

} else if !datastore.ErrorIsNotFound(err) {

return err

}

_, err = osc.ds.GetPersistentVolume(store.Name)

if err == nil {

osc.ds.DeletePersistentVolume(store.Name)

} else if !datastore.ErrorIsNotFound(err) {

return err

}

_, err = osc.ds.GetVolume(store.Name)

if err == nil {

osc.ds.DeleteVolume(store.Name)

} else if !datastore.ErrorIsNotFound(err) {

return err

}

// cleanup all secrets with matching labels.

labels := types.GetBaseLabelsForSystemManagedComponent()

labels[types.GetLonghornLabelComponentKey()] = types.LonghornLabelObjectStore

labels[types.GetLonghornLabelKey(types.LonghornLabelObjectStore)] = store.Name

secrets, err := osc.ds.ListSecretsByLabels(osc.namespace, labels)

if err != nil {

return err

}

for _, secret := range secrets {

osc.ds.DeleteSecret(osc.namespace, secret.Name)

}

if len(store.ObjectMeta.Finalizers) != 0 {

return osc.ds.RemoveFinalizerForObjectStore(store)

}

return nil

}

func (osc *ObjectStoreController) handleTerminating(store *longhorn.ObjectStore) (err error) {

// The resources are created in the order:

// Volume -> PV -> PVC -> Deployment -> Service

// so we tear them down in reverse:

// Service -> Deployment -> PVC -> PV -> Volume

// Once that is done we can remove the finalizer, which allows the K8s API to

// remove the object store resource as well.

if err := osc.ds.DeleteService(osc.namespace, store.Name); err != nil && !datastore.ErrorIsNotFound(err) {

return err

}

if err := osc.ds.DeleteDeployment(store.Name); err != nil && !datastore.ErrorIsNotFound(err) {

return err

}

if err := osc.ds.DeletePersistentVolumeClaim(osc.namespace, store.Name); err != nil && !datastore.ErrorIsNotFound(err) {

return err

}

if err := osc.ds.DeletePersistentVolume(store.Name); err != nil && !datastore.ErrorIsNotFound(err) {

return err

}

if err := osc.ds.DeleteVolume(store.Name); err != nil && !datastore.ErrorIsNotFound(err) {

return err

}

// cleanup all secrets with matching labels.

labels := types.GetBaseLabelsForSystemManagedComponent()

labels[types.GetLonghornLabelComponentKey()] = types.LonghornLabelObjectStore

labels[types.GetLonghornLabelKey(types.LonghornLabelObjectStore)] = store.Name

secrets, err := osc.ds.ListSecretsByLabels(osc.namespace, labels)

if err != nil {

return err

}

for _, secret := range secrets {

if err := osc.ds.DeleteSecret(osc.namespace, secret.Name); err != nil && !datastore.ErrorIsNotFound(err) {

return err

}

}

return osc.ds.RemoveFinalizerForObjectStore(store)

}

PhanLe1010

In general LGTM

PhanLe1010 · 2023-11-29T23:25:19Z

In the future upgrade, if we want to change the default manifests for pv/pvc/dpl/endpoints/svc/secrets, what will we do?

There is currently nothing planned for that case, since it's unclear to me what situations exactly can occur and I don't expect the manifests to change a lot. We can easily change the manifests that are hard-coded in the controller to match the corresponding version of the s3gw. This works for all newly deployed object stores. But if you're worried about upgrading existing object stores, the story gets much more complicated. We would essentially need versioned manifests.

Can you describe what situation exactly you're thinking about here?

Thanks @shuo-wu for the good catch.

I think we should create a new ticket for handling this upgrading design.

One of the most common use-case is that:

User deploys Longhorn v1.6.0 which comes with manifest compatible with s3gw v0.23.0
User upgrade to Longhorn v1.6.1, which comes with manifest compatible with s3gw v0.24.0
Now that s3gw v0.24.0 has some critical fixes since v0.23.0, the user wants to Upgrade ObjectStore to s3gw v0.24.0.
We cannot just replace the image of ObjectStore deployment since the current deployment manifest was created since Longhorn v1.6.0 and it might not be compatible with s3gw v0.24.0

Another use case is that:

User deploys Longhorn v1.6.0 which comes with manifest compatible with s3gw v0.23.0
User takes a backup of the ObjectStore
User upgrade to Longhorn v1.6.1, which comes with new manifest compatible with s3gw v0.24.0
User restores the ObjectStore from the backup to a new ObjectStore.
Longhorn v1.6.1 cannot blindly use the manifest compatible with s3gw v0.24.0 for restored ObjectStore with image s3gw v0.23.0

shuo-wu · 2023-11-30T09:02:08Z

I don't think the resource upgrade should rely on the backup & restore feature. I am thinking if we can re-deploy everything if users try to upgrade the ObjectStore.spec.image. Since the deployment does not support rolling upgrade, re-deploying those components should be fine.

Simplify deletes by removing preceeding gets and dealing with errors. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

mergify · 2023-12-01T09:12:41Z

This pull request is now in conflicts. Could you fix it @m-ildefons? 🙏

This was referenced Sep 6, 2023

Object Storage (LEP 20230430) longhorn/longhorn-ui#649

Closed

Object Storage (LEP 20230430) longhorn/longhorn#6640

Closed

[Draft] [DNM] Object Storage longhorn/longhorn-tests#1523

Closed

m-ildefons force-pushed the wip/object-endpoint branch 2 times, most recently from ebd3e0e to a82b8aa Compare September 6, 2023 14:02

PhanLe1010 reviewed Sep 7, 2023

View reviewed changes

k8s/pkg/apis/longhorn/v1beta2/objectstore.go Outdated Show resolved Hide resolved

PhanLe1010 reviewed Sep 7, 2023

View reviewed changes

k8s/pkg/apis/longhorn/v1beta2/objectstore.go Outdated Show resolved Hide resolved

m-ildefons force-pushed the wip/object-endpoint branch from 765a552 to 659174d Compare September 11, 2023 15:17

m-ildefons force-pushed the wip/object-endpoint branch from 25be09a to 204390f Compare September 19, 2023 08:36

m-ildefons mentioned this pull request Sep 19, 2023

Longhorn controller (Epic) s3gw-tech/s3gw#470

Closed

17 tasks

m-ildefons force-pushed the wip/object-endpoint branch from f94f721 to a1daec8 Compare September 21, 2023 12:25

m-ildefons changed the title ~~[Draft] [DNM] Object Storage~~ Object Storage (LEP 20230430) Sep 28, 2023

m-ildefons marked this pull request as ready for review September 28, 2023 15:25

m-ildefons requested a review from a team as a code owner September 28, 2023 15:25

PhanLe1010 reviewed Oct 24, 2023

View reviewed changes

k8s/pkg/apis/longhorn/v1beta2/objectstore.go Show resolved Hide resolved

controller/object_store_controller.go Show resolved Hide resolved

k8s/pkg/apis/longhorn/v1beta2/objectstore.go Show resolved Hide resolved

controller/object_store_controller.go Outdated Show resolved Hide resolved

PhanLe1010 reviewed Oct 24, 2023

View reviewed changes

controller/object_store_controller.go Outdated Show resolved Hide resolved

controller/object_store_controller.go Outdated Show resolved Hide resolved

PhanLe1010 reviewed Oct 24, 2023

View reviewed changes

PhanLe1010 reviewed Oct 27, 2023

View reviewed changes

controller/object_store_controller.go Outdated Show resolved Hide resolved

k8s/pkg/apis/longhorn/v1beta2/objectstore.go Show resolved Hide resolved

PhanLe1010 reviewed Oct 27, 2023

View reviewed changes

controller/object_store_controller.go Outdated Show resolved Hide resolved

PhanLe1010 reviewed Oct 27, 2023

View reviewed changes

controller/object_store_controller.go Outdated Show resolved Hide resolved

PhanLe1010 reviewed Oct 30, 2023

View reviewed changes

m-ildefons added 19 commits November 28, 2023 17:33

object store: fixes for review

8dfdfb4

Fix review remarks Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

object store: fix event trigger

9bf594a

Fix event trigger for PVC events. This had misakenly been wired up to call the event handler for Service events, but now it's fixed to call the right event handler. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

object store: fix secret creation

fad0bd0

Fix creating a secret when given credentials from the UI. Instead of over-writing an existing secret or similar antics, generate a name without a conflict for a new secret to use. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

object store: safe image modifications

53750c6

Update CRDs Safe image modifications when updating images Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

object store: fix typo

7065db2

Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

m-ildefons force-pushed the wip/object-endpoint branch from 3046278 to 1a93355 Compare November 29, 2023 09:46

PhanLe1010 reviewed Nov 29, 2023

View reviewed changes

PhanLe1010 previously approved these changes Nov 29, 2023

View reviewed changes

object store: simplify deletes

cbaa80e

Simplify deletes by removing preceeding gets and dealing with errors. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>

m-ildefons dismissed PhanLe1010’s stale review via cbaa80e December 1, 2023 08:16

m-ildefons closed this Dec 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Object Storage (LEP 20230430) #2136

Object Storage (LEP 20230430) #2136

m-ildefons commented Sep 6, 2023 •

edited

Loading

mergify bot commented Sep 6, 2023

mergify bot commented Sep 12, 2023

mergify bot commented Oct 6, 2023

m-ildefons commented Nov 29, 2023

PhanLe1010 Nov 29, 2023

PhanLe1010 Nov 29, 2023

PhanLe1010 left a comment

PhanLe1010 commented Nov 29, 2023

shuo-wu commented Nov 30, 2023

mergify bot commented Dec 1, 2023

Object Storage (LEP 20230430) #2136

Object Storage (LEP 20230430) #2136

Conversation

m-ildefons commented Sep 6, 2023 • edited Loading

mergify bot commented Sep 6, 2023

mergify bot commented Sep 12, 2023

mergify bot commented Oct 6, 2023

m-ildefons commented Nov 29, 2023

PhanLe1010 Nov 29, 2023

Choose a reason for hiding this comment

PhanLe1010 Nov 29, 2023

Choose a reason for hiding this comment

PhanLe1010 left a comment

Choose a reason for hiding this comment

PhanLe1010 commented Nov 29, 2023

shuo-wu commented Nov 30, 2023

mergify bot commented Dec 1, 2023

m-ildefons commented Sep 6, 2023 •

edited

Loading