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

Stop deploying memcached automatically #145

Merged
merged 1 commit into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions api/v1beta1/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import (
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
)

// Horizon Condition Types used by API objects.
const (
// HorizonMemcachedError - Provides a error that occured during the provisioning of the memcached instance
HorizonMemcachedError = "Error creating Memcached instance: %s"

// HorizonMemcachedReadyCondition - Indicates the Horizon memcached service is ready to be consumed
// by Horizon
// HorizonMemcachedReadyCondition - Indicates the Horizon memcached service is ready to be consumed by Horizon
HorizonMemcachedReadyCondition condition.Type = "HorizonMemcached"
)

// Horizon Messages used by API objects.
const (
// HorizonMemcachedReadyInitMessage -
HorizonMemcachedReadyInitMessage = "Horizon Memcached create not started"

Expand Down
8 changes: 4 additions & 4 deletions api/v1beta1/horizon_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ type HorizonSpec struct {
//TODO(bshephar) Implement everything about this. It's just a placeholder at the moment.
Route HorizonRoute `json:"route,omitempty"`

// +kubebuilder:validation:Optional
// SharedMemcahed holds the name of the central memcached instance if it exists. If this value is provided,
// then Horizon will use the shared memcached service. Otherwise, we will create one just for Horizon.
SharedMemcached string `json:"sharedMemcached,omitempty"`
// +kubebuilder:validation:Required
// +kubebuilder:default=memcached
// Memcached instance name.
MemcachedInstance string `json:"memcachedInstance"`
}

// HorizonDebug can be used to enable debug in the Horizon service
Expand Down
11 changes: 5 additions & 6 deletions config/crd/bases/horizon.openstack.org_horizons.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ spec:
to add additional files. Those get added to the service config dir
in /etc/<service> . TODO: -> implement'
type: object
memcachedInstance:
default: memcached
description: Memcached instance name.
type: string
nodeSelector:
additionalProperties:
type: string
Expand Down Expand Up @@ -169,14 +173,9 @@ spec:
description: Secret containing OpenStack password information for
Horizon Secret Key
type: string
sharedMemcached:
description: SharedMemcahed holds the name of the central memcached
instance if it exists. If this value is provided, then Horizon will
use the shared memcached service. Otherwise, we will create one
just for Horizon.
type: string
required:
- containerImage
- memcachedInstance
- secret
type: object
status:
Expand Down
7 changes: 0 additions & 7 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,6 @@ rules:
- patch
- update
- watch
- apiGroups:
- ""
resources:
- pods
verbs:
- get
- list
- apiGroups:
- ""
resources:
Expand Down
53 changes: 8 additions & 45 deletions controllers/horizon_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import (
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/rand"
Expand Down Expand Up @@ -90,7 +89,6 @@ type HorizonReconciler struct {
//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete;
//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch;delete;
//+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;create;update;patch;delete;
//+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;
//+kubebuilder:rbac:groups=route.openshift.io,resources=routes,verbs=get;list;watch;create;update;patch;delete;
//+kubebuilder:rbac:groups=keystone.openstack.org,resources=keystoneapis,verbs=get;list;watch;
//+kubebuilder:rbac:groups=keystone.openstack.org,resources=keystoneendpoints,verbs=get;list;watch;
Expand Down Expand Up @@ -212,7 +210,6 @@ func (r *HorizonReconciler) SetupWithManager(mgr ctrl.Manager) error {
Owns(&corev1.ConfigMap{}).
Owns(&appsv1.Deployment{}).
Owns(&routev1.Route{}).
Owns(&memcachedv1.Memcached{}).
Complete(r)
}

Expand Down Expand Up @@ -340,22 +337,18 @@ func (r *HorizonReconciler) reconcileNormal(ctx context.Context, instance *horiz
instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
// run check OpenStack secret - end

// Create Memcached instance if no shared instance exists.
var memcached *memcachedv1.Memcached
if instance.Spec.SharedMemcached == "" {
memcached, err = r.ensureHorizonMemcached(ctx, helper, instance)
} else {
memcached, err = r.getSharedMemcached(ctx, helper, instance)
}

//
// Check for required memcached used for caching
//
memcached, err := r.getHorizonMemcached(ctx, helper, instance)
kajinamit marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
if k8s_errors.IsNotFound(err) {
instance.Status.Conditions.Set(condition.FalseCondition(
horizonv1beta1.HorizonMemcachedReadyCondition,
condition.RequestedReason,
condition.SeverityInfo,
horizonv1beta1.HorizonMemcachedReadyWaitingMessage))
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("memcached %s not found", instance.Spec.SharedMemcached)
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("memcached %s not found", instance.Spec.MemcachedInstance)
}
instance.Status.Conditions.Set(condition.FalseCondition(
horizonv1beta1.HorizonMemcachedReadyCondition,
Expand All @@ -377,6 +370,7 @@ func (r *HorizonReconciler) reconcileNormal(ctx context.Context, instance *horiz
// Mark the Memcached Service as Ready if we get to this point with no errors
instance.Status.Conditions.MarkTrue(
horizonv1beta1.HorizonMemcachedReadyCondition, horizonv1beta1.HorizonMemcachedReadyMessage)
// run check memcached - end

//
// Create ConfigMaps and Secrets required as input for the Service and calculate an overall hash of hashes
Expand Down Expand Up @@ -622,38 +616,7 @@ func (r *HorizonReconciler) ensureHorizonSecret(
return nil
}

func (r *HorizonReconciler) ensureHorizonMemcached(
ctx context.Context,
h *helper.Helper,
instance *horizonv1beta1.Horizon,
) (*memcachedv1.Memcached, error) {
memcached := &memcachedv1.Memcached{
TypeMeta: metav1.TypeMeta{
APIVersion: "memcached.openstack.org/v1beta1",
Kind: "Memcached",
},
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-memcached", instance.Name),
Namespace: instance.Namespace,
},
}

op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), memcached, func() error {
memcached.Labels = map[string]string{"service": horizon.ServiceName}
memcached.Spec.Replicas = instance.Spec.Replicas
return controllerutil.SetControllerReference(h.GetBeforeObject(), memcached, h.GetScheme())
})

if err != nil {
return nil, err
}
if op != controllerutil.OperationResultNone {
r.Log.Info(fmt.Sprintf("Memcached %s - %s", memcached.Name, op))
}
return memcached, nil
}

func (r *HorizonReconciler) getSharedMemcached(
func (r *HorizonReconciler) getHorizonMemcached(
ctx context.Context,
h *helper.Helper,
instance *horizonv1beta1.Horizon,
Expand All @@ -662,7 +625,7 @@ func (r *HorizonReconciler) getSharedMemcached(
err := h.GetClient().Get(
ctx,
types.NamespacedName{
Name: instance.Spec.SharedMemcached,
Name: instance.Spec.MemcachedInstance,
Namespace: instance.Namespace,
},
memcached)
Expand Down
29 changes: 13 additions & 16 deletions tests/functional/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ func DefaultHorizonTemplate(name types.NamespacedName) *horizon.Horizon {
Namespace: name.Namespace,
},
Spec: horizon.HorizonSpec{
ContainerImage: "test-horizon-container-image",
Secret: SecretName,
ContainerImage: "test-horizon-container-image",
Secret: SecretName,
MemcachedInstance: "memcached",
},
}
}
Expand All @@ -64,34 +65,30 @@ func GetHorizon(name types.NamespacedName) *horizon.Horizon {
return instance
}

func CreateHorizonSharedMemcached(name types.NamespacedName) *horizon.Horizon {
instance := DefaultHorizonTemplate(name)
// Set shared Memcached
instance.Spec.SharedMemcached = "shared-memcached"
err := k8sClient.Create(ctx, instance)
Expect(err).NotTo(HaveOccurred())

return instance
}

func SharedMemcached() *memcachedv1.Memcached {
func HorizonMemcached() *memcachedv1.Memcached {
return &memcachedv1.Memcached{
TypeMeta: metav1.TypeMeta{
APIVersion: "memcached.openstack.org/v1beta1",
Kind: "Memcached",
},
ObjectMeta: metav1.ObjectMeta{
Name: "shared-memcached",
Name: "memcached",
Namespace: namespace,
},
}
}

func CreateSharedMemcached() *memcachedv1.Memcached {
instance := SharedMemcached()
func CreateHorizonMemcached() *memcachedv1.Memcached {
instance := HorizonMemcached()
err := k8sClient.Create(ctx, instance)
Expect(err).NotTo(HaveOccurred())

instance.Status.Conditions.MarkTrue(condition.ReadyCondition, condition.ReadyMessage)
instance.Status.ReadyCount = int32(1)
instance.Status.ServerList = []string{"memcached-0.memcached:11211"}
instance.Status.ServerListWithInet = []string{"inet:[memcached-0.memcached]:11211"}
Expect(k8sClient.Status().Update(ctx, instance)).Should(Succeed())

return instance
}

Expand Down
44 changes: 13 additions & 31 deletions tests/functional/horizon_controller_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package functional_test

import (
"fmt"
"os"

. "github.com/onsi/ginkgo/v2"
Expand All @@ -12,24 +11,20 @@ import (

condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
. "github.com/openstack-k8s-operators/lib-common/modules/test/helpers"

horizonv1beta1 "github.com/openstack-k8s-operators/horizon-operator/api/v1beta1"
)

var _ = Describe("Horizon controller", func() {

var horizonName types.NamespacedName
var memcachedName types.NamespacedName
var secret *corev1.Secret

BeforeEach(func() {

horizonName = types.NamespacedName{
Name: "horizon",
Namespace: namespace,
}
memcachedName = types.NamespacedName{
Name: fmt.Sprintf("%s-memcached", horizonName.Name),
Namespace: horizonName.Namespace,
}

// lib-common uses OPERATOR_TEMPLATES env var to locate the "templates"
// directory of the operator. We need to set them othervise lib-common
Expand Down Expand Up @@ -94,7 +89,7 @@ var _ = Describe("Horizon controller", func() {
BeforeEach(func() {
DeferCleanup(DeleteInstance, CreateHorizon(horizonName))
})
It("should not be in a state of having the input ready", func() {
It("should be in a state of having the input ready", func() {
secret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: SecretName,
Expand All @@ -110,28 +105,11 @@ var _ = Describe("Horizon controller", func() {
)
})
})
When("Using dedicated memcached", func() {
BeforeEach(func() {
DeferCleanup(DeleteInstance, CreateHorizon(horizonName))
secret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: SecretName,
Namespace: namespace,
},
}
Expect(k8sClient.Create(ctx, secret)).Should(Succeed())
})
It("Should create a memcached deployment with service label", func() {
memcached := GetMemcached(memcachedName)
instance := GetHorizon(horizonName)
Expect(memcached.Labels["service"]).Should(Equal("horizon"))
Expect(memcached.Spec.Replicas).Should(Equal(instance.Spec.Replicas))
})
})

When("using a shared memcached instance", func() {
BeforeEach(func() {
DeferCleanup(DeleteInstance, CreateSharedMemcached())
DeferCleanup(DeleteInstance, CreateHorizonSharedMemcached(horizonName))
DeferCleanup(DeleteInstance, CreateHorizon(horizonName))
DeferCleanup(DeleteInstance, CreateHorizonMemcached())
secret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: SecretName,
Expand All @@ -140,9 +118,13 @@ var _ = Describe("Horizon controller", func() {
}
Expect(k8sClient.Create(ctx, secret)).Should(Succeed())
})
It("Should find the shared-memcached instance", func() {
memcached := GetMemcached(types.NamespacedName{Namespace: namespace, Name: "shared-memcached"})
Expect(memcached).NotTo(BeNil())
It("should be in a state of having the memcached ready", func() {
th.ExpectCondition(
horizonName,
ConditionGetterFunc(HorizonConditionGetter),
horizonv1beta1.HorizonMemcachedReadyCondition,
corev1.ConditionTrue,
)
})
})
})
7 changes: 0 additions & 7 deletions tests/kuttl/common/assert-sample-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,3 @@ spec:
SESSION_TIMEOUT = 3600
status:
readyCount: 1
---
apiVersion: memcached.openstack.org/v1beta1
kind: Memcached
metadata:
name: horizon-memcached
spec:
replicas: 1
Loading