Skip to content

Commit

Permalink
Refactor ring device setup
Browse files Browse the repository at this point in the history 
Creates device list now within the SwiftRing instance instead of the
SwiftStorage instance. This is preliminary work to add device setup for
dataplane nodes in the SwiftRing instance as well.

This cleans up the SwiftStorage instance quite a bit and moves ring
management code into the SwiftRing instance.

Also change the logic: the ring rebalance job is now only executed if
the deviceList hash did change, changes to the Swift ring file ConfigMap
no longer trigger a rebalance.
  • Loading branch information
@cschwede
cschwede committed Feb 27, 2024
1 parent fb2f23c commit 1538a96
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 117 deletions.
73 changes: 37 additions & 36 deletions controllers/swiftring_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,49 +151,55 @@ func (r *SwiftRingReconciler) reconcileNormal(ctx context.Context, instance *swi

serviceLabels := swiftring.Labels()

// Create a ConfigMap populated with content from templates/
envVars := make(map[string]env.Setter)
tpl := swiftring.ConfigMapTemplates(instance, serviceLabels)
err := configmap.EnsureConfigMaps(ctx, helper, instance, tpl, &envVars)
if err != nil {
return ctrl.Result{}, err
}

// Swift ring init job - start
if instance.Status.Hash == nil {
instance.Status.Hash = map[string]string{}
}
ringCreateHash := instance.Status.Hash[swiftv1beta1.RingCreateHash]

// Check if the device list ConfigMap did change and if so, delete the
// rebalance Job. This will result in a new Job that rebalances with
// the updated device list
_, deviceListHash, err := configmap.GetConfigMapAndHashWithName(ctx, helper, swift.DeviceConfigMapName, instance.Namespace)
deviceList, deviceListHash, err := swiftring.DeviceList(ctx, helper, instance)
if err != nil {
return ctrl.Result{}, err
}

if instance.Status.Hash[swiftv1beta1.DeviceListHash] != deviceListHash {
if err := job.DeleteJob(ctx, helper, instance.Name+"-rebalance", instance.Namespace); err != nil {
// Create or update the devicelist ConfigMap
envVars := make(map[string]env.Setter)
tpl := swiftring.ConfigMapTemplates(instance, serviceLabels, deviceList)
err = configmap.EnsureConfigMaps(ctx, helper, instance, tpl, &envVars)
if err != nil {
return ctrl.Result{}, err
}

// Delete a possibly still existing job that finished to re-run the job
j, err := job.GetJobWithName(ctx, helper, instance.Name+"-rebalance", instance.Namespace)
if err != nil {
if !apierrors.IsNotFound(err) {
return ctrl.Result{}, err
}
} else {
if j.Status.Active == 0 {
err = job.DeleteJob(ctx, helper, instance.Name+"-rebalance", instance.Namespace)
if err != nil {
return ctrl.Result{}, err
}
}
}

instance.Status.Hash[swiftv1beta1.RingCreateHash] = ""
instance.Status.Hash[swiftv1beta1.DeviceListHash] = deviceListHash
if err := r.Status().Update(ctx, instance); err != nil {
return ctrl.Result{}, err
}
}

ringCreateJob := job.NewJob(swiftring.GetRingJob(instance, serviceLabels), swiftv1beta1.RingCreateHash, false, 5*time.Second, ringCreateHash)
ringCreateJob := job.NewJob(swiftring.GetRingJob(instance, serviceLabels), "rebalance", false, 5*time.Second, instance.Status.Hash[swiftv1beta1.RingCreateHash])
ctrlResult, err := ringCreateJob.DoJob(ctx, helper)
if (ctrlResult != ctrl.Result{}) {
instance.Status.Conditions.Set(condition.FalseCondition(
condition.ReadyCondition,
condition.RequestedReason,
condition.SeverityInfo,
condition.ReadyInitMessage))
if err := r.Status().Update(ctx, instance); err != nil {
return ctrl.Result{}, err
}
return ctrlResult, nil
}
if err != nil {
Expand All @@ -210,7 +216,6 @@ func (r *SwiftRingReconciler) reconcileNormal(ctx context.Context, instance *swi

if ringCreateJob.HasChanged() {
instance.Status.Hash[swiftv1beta1.RingCreateHash] = ringCreateJob.GetHash()
instance.Status.Hash[swiftv1beta1.DeviceListHash] = deviceListHash
if err := r.Status().Update(ctx, instance); err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -255,24 +260,20 @@ func (r *SwiftRingReconciler) reconcileDelete(ctx context.Context, instance *swi

// SetupWithManager sets up the controller with the Manager.
func (r *SwiftRingReconciler) SetupWithManager(mgr ctrl.Manager) error {
deviceConfigMapFilter := func(ctx context.Context, o client.Object) []reconcile.Request {
swiftRingFilter := func(ctx context.Context, o client.Object) []reconcile.Request {
result := []reconcile.Request{}
if o.GetName() == swift.DeviceConfigMapName {
// There should be only one SwiftRing instance within
// the Namespace - that needs to be reconciled
swiftRings := &swiftv1beta1.SwiftRingList{}
listOpts := []client.ListOption{client.InNamespace(o.GetNamespace())}
err := r.Client.List(context.Background(), swiftRings, listOpts...)
if err != nil {
return nil
}
for _, cr := range swiftRings.Items {
name := client.ObjectKey{
Namespace: o.GetNamespace(),
Name: cr.Name,
}
result = append(result, reconcile.Request{NamespacedName: name})
swiftRings := &swiftv1beta1.SwiftRingList{}
listOpts := []client.ListOption{client.InNamespace(o.GetNamespace())}
err := r.Client.List(context.Background(), swiftRings, listOpts...)
if err != nil {
return nil
}
for _, cr := range swiftRings.Items {
name := client.ObjectKey{
Namespace: o.GetNamespace(),
Name: cr.Name,
}
result = append(result, reconcile.Request{NamespacedName: name})
}
return result
}
Expand All @@ -283,6 +284,6 @@ func (r *SwiftRingReconciler) SetupWithManager(mgr ctrl.Manager) error {
Owns(&corev1.ConfigMap{}).
Owns(&rbacv1.ClusterRole{}).
Owns(&rbacv1.ClusterRoleBinding{}).
Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(deviceConfigMapFilter)).
Watches(&swiftv1beta1.SwiftStorage{}, handler.EnqueueRequestsFromMapFunc(swiftRingFilter)).
Complete(r)
}
27 changes: 1 addition & 26 deletions controllers/swiftstorage_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import (
"k8s.io/client-go/kubernetes"

swiftv1beta1 "github.com/openstack-k8s-operators/swift-operator/api/v1beta1"
"github.com/openstack-k8s-operators/swift-operator/pkg/swift"
"github.com/openstack-k8s-operators/swift-operator/pkg/swiftstorage"

"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
Expand Down Expand Up @@ -141,22 +140,6 @@ func (r *SwiftStorageReconciler) Reconcile(ctx context.Context, req ctrl.Request
serviceLabels := swiftstorage.Labels()
envVars := make(map[string]env.Setter)

// Check if there is already an existing ConfigMap and device list. If
// not, create an initial device list to bootstrap the cluster with The
// weights are simply set to the requested size, this will be changed
// once all StatefulSets are running
_, ctrlResult, err := configmap.GetConfigMap(ctx, helper, instance, swift.DeviceConfigMapName, 5*time.Second)
if err != nil {
return ctrlResult, err
} else if (ctrlResult != ctrl.Result{}) {
devices := swiftstorage.DeviceList(ctx, helper, instance)
tpl := swiftstorage.DeviceConfigMapTemplates(instance, devices)
err = configmap.EnsureConfigMaps(ctx, helper, instance, tpl, &envVars)
if err != nil {
return ctrl.Result{}, err
}
}

// Create a ConfigMap populated with content from templates/
tpl := swiftstorage.ConfigMapTemplates(instance, serviceLabels, instance.Spec.MemcachedServers)
err = configmap.EnsureConfigMaps(ctx, helper, instance, tpl, &envVars)
Expand All @@ -169,7 +152,7 @@ func (r *SwiftStorageReconciler) Reconcile(ctx context.Context, req ctrl.Request
if err != nil {
return ctrl.Result{}, err
}
ctrlResult, err = svc.CreateOrPatch(ctx, helper)
ctrlResult, err := svc.CreateOrPatch(ctx, helper)
if err != nil {
return ctrlResult, err
} else if (ctrlResult != ctrl.Result{}) {
Expand Down Expand Up @@ -236,14 +219,6 @@ func (r *SwiftStorageReconciler) Reconcile(ctx context.Context, req ctrl.Request

instance.Status.ReadyCount = sset.GetStatefulSet().Status.ReadyReplicas
if instance.Status.ReadyCount == *instance.Spec.Replicas {
envVars := make(map[string]env.Setter)
devices := swiftstorage.DeviceList(ctx, helper, instance)
tpl = swiftstorage.DeviceConfigMapTemplates(instance, devices)
err = configmap.EnsureConfigMaps(ctx, helper, instance, tpl, &envVars)
if err != nil {
return ctrl.Result{}, err
}

// When the cluster is attached to an external network, create DNS record for every
// cluster member so it can be resolved from outside cluster (edpm nodes)
podList, err := pod.GetPodListWithLabel(ctx, helper, instance.Namespace, serviceLabels)
Expand Down
64 changes: 64 additions & 0 deletions pkg/swiftring/funcs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,74 @@ limitations under the License.
package swiftring

import (
"context"
"fmt"
"sort"
"strings"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/openstack-k8s-operators/lib-common/modules/common"
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
"github.com/openstack-k8s-operators/lib-common/modules/common/util"

swiftv1beta1 "github.com/openstack-k8s-operators/swift-operator/api/v1beta1"
"github.com/openstack-k8s-operators/swift-operator/pkg/swift"
)

//+kubebuilder:rbac:groups=core,resources=persistentvolumeclaims,verbs=get;list;watch

func DeviceList(ctx context.Context, h *helper.Helper, instance *swiftv1beta1.SwiftRing) (string, string, error) {
// Returns a list of devices as CSV
devices := []string{}

listOpts := []client.ListOption{client.InNamespace(instance.GetNamespace())}

// Get all SwiftStorage instances
storages := &swiftv1beta1.SwiftStorageList{}
err := h.GetClient().List(context.Background(), storages, listOpts...)
if err != nil && !errors.IsNotFound(err) {
return "", "", err
}
for _, storageInstance := range storages.Items {
for replica := 0; replica < int(*storageInstance.Spec.Replicas); replica++ {
cn := fmt.Sprintf("%s-%s-%d", swift.ClaimName, storageInstance.Name, replica)
foundClaim := &corev1.PersistentVolumeClaim{}
err = h.GetClient().Get(ctx, types.NamespacedName{Name: cn, Namespace: storageInstance.Namespace}, foundClaim)
capacity := resource.MustParse(storageInstance.Spec.StorageRequest)
weight, _ := capacity.AsInt64()
if err == nil {
capacity := foundClaim.Status.Capacity["storage"]
weight, _ = capacity.AsInt64()
} else {
h.GetLogger().Info(fmt.Sprintf("Did not find PVC %s, assuming %s as capacity", cn, storageInstance.Spec.StorageRequest))
}
weight = weight / (1000 * 1000 * 1000) // 10GiB gets a weight of 10 etc.
// CSV: region,zone,hostname,devicename,weight
devices = append(devices, fmt.Sprintf("1,1,%s-%d.%s,%s,%d\n", storageInstance.Name, replica, storageInstance.Name, "d1", weight))
}
}

// Device list must be sorted to ensure hash does not change
sort.Strings(devices)

var deviceList strings.Builder
for _, line := range devices {
deviceList.WriteString(line)
}

deviceListHash, err := util.ObjectHash(deviceList.String())
if err != nil {
return "", "", err
}

return deviceList.String(), deviceListHash, nil
}

func Labels() map[string]string {
return map[string]string{
common.AppSelector: swift.ServiceName,
Expand Down
13 changes: 12 additions & 1 deletion pkg/swiftring/templates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,21 @@ import (
"fmt"
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
swiftv1beta1 "github.com/openstack-k8s-operators/swift-operator/api/v1beta1"
"github.com/openstack-k8s-operators/swift-operator/pkg/swift"
)

func ConfigMapTemplates(instance *swiftv1beta1.SwiftRing, labels map[string]string) []util.Template {
func ConfigMapTemplates(instance *swiftv1beta1.SwiftRing, labels map[string]string, devices string) []util.Template {
data := make(map[string]string)
data["devices.csv"] = devices

return []util.Template{
{
Name: swift.DeviceConfigMapName,
Namespace: instance.Namespace,
Type: util.TemplateTypeNone,
InstanceType: instance.Kind,
CustomData: data,
},
{
Name: fmt.Sprintf("%s-scripts", instance.Name),
Namespace: instance.Namespace,
Expand Down
38 changes: 0 additions & 38 deletions pkg/swiftstorage/funcs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,48 +17,10 @@ limitations under the License.
package swiftstorage

import (
"context"
"fmt"
"strings"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/types"

"github.com/openstack-k8s-operators/lib-common/modules/common"
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"

swiftv1beta1 "github.com/openstack-k8s-operators/swift-operator/api/v1beta1"
"github.com/openstack-k8s-operators/swift-operator/pkg/swift"
)

//+kubebuilder:rbac:groups=core,resources=persistentvolumeclaims,verbs=get;list;watch

func DeviceList(ctx context.Context, h *helper.Helper, instance *swiftv1beta1.SwiftStorage) string {
// Creates a CSV list of devices. If PVCs do not exist yet (because not
// all StatefulSets are up yet), it will just use the request capacity
// as value.
var devices strings.Builder

foundClaim := &corev1.PersistentVolumeClaim{}
for replica := 0; replica < int(*instance.Spec.Replicas); replica++ {
cn := fmt.Sprintf("%s-%s-%d", swift.ClaimName, instance.Name, replica)
err := h.GetClient().Get(ctx, types.NamespacedName{Name: cn, Namespace: instance.Namespace}, foundClaim)
capacity := resource.MustParse(instance.Spec.StorageRequest)
weight, _ := capacity.AsInt64()
if err == nil {
capacity := foundClaim.Status.Capacity["storage"]
weight, _ = capacity.AsInt64()
} else {
h.GetLogger().Info(fmt.Sprintf("Did not find PVC %s, assuming %s as capacity", cn, instance.Spec.StorageRequest))
}
weight = weight / (1000 * 1000 * 1000) // 10GiB gets a weight of 10 etc.
// CSV: region,zone,hostname,devicename,weight
devices.WriteString(fmt.Sprintf("1,1,%s-%d.%s.%s.svc,%s,%d\n", instance.Name, replica, instance.Name, instance.Namespace, "d1", weight))
}
return devices.String()
}

func Labels() map[string]string {
return map[string]string{
common.AppSelector: swift.ServiceName,
Expand Down
16 changes: 0 additions & 16 deletions pkg/swiftstorage/templates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"fmt"
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
swiftv1beta1 "github.com/openstack-k8s-operators/swift-operator/api/v1beta1"
"github.com/openstack-k8s-operators/swift-operator/pkg/swift"
)

func ConfigMapTemplates(instance *swiftv1beta1.SwiftStorage, labels map[string]string, memcachedServers string) []util.Template {
Expand Down Expand Up @@ -52,18 +51,3 @@ func ConfigMapTemplates(instance *swiftv1beta1.SwiftStorage, labels map[string]s
},
}
}

func DeviceConfigMapTemplates(instance *swiftv1beta1.SwiftStorage, devices string) []util.Template {
data := make(map[string]string)
data["devices.csv"] = devices

return []util.Template{
{
Name: swift.DeviceConfigMapName,
Namespace: instance.Namespace,
Type: util.TemplateTypeNone,
InstanceType: instance.Kind,
CustomData: data,
},
}
}

0 comments on commit 1538a96

Please sign in to comment.