Skip to content

Commit

Permalink
[artemiscloud#362] - check for console.expose services that don't hav…
Browse files Browse the repository at this point in the history 
…e an owner reference and reclaim
  • Loading branch information
@gtully @michalxo
gtully authored and michalxo committed Nov 2, 2022
1 parent fbe6859 commit de06e2f
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 60 deletions.
35 changes: 33 additions & 2 deletions controllers/activemqartemis_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"strconv"
"strings"

"github.com/artemiscloud/activemq-artemis-operator/api/v1beta1"
brokerv2alpha4 "github.com/artemiscloud/activemq-artemis-operator/api/v2alpha4"
"github.com/artemiscloud/activemq-artemis-operator/api/v2alpha5"
"github.com/artemiscloud/activemq-artemis-operator/pkg/resources/environments"
Expand Down Expand Up @@ -794,7 +793,7 @@ var _ = Describe("artemis controller", func() {
}, timeout, interval).Should(Succeed())

key = types.NamespacedName{Name: toCreate.Name, Namespace: toCreate.Namespace}
createdCrd := &v1beta1.ActiveMQArtemis{}
createdCrd := &brokerv1beta1.ActiveMQArtemis{}
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, key, createdCrd)).Should(Succeed())
g.Expect(len(createdCrd.Status.PodStatus.Stopped)).Should(BeEquivalentTo(1))
Expand Down Expand Up @@ -959,6 +958,26 @@ var _ = Describe("artemis controller", func() {
TimeoutSeconds: 5,
}

// an eariler operator would not set an owner ref on services and we can clash
// on create when trying to create a service with the same name..
// verify that we find and fix the owner ref before reconcile
var dudPort = int32(22)
serviceKey := types.NamespacedName{Name: crd.Name + "-wconsj-0-svc", Namespace: defaultNamespace}
existingSrviceWithOutOwnerRef := corev1.Service{
ObjectMeta: metav1.ObjectMeta{Name: serviceKey.Name, Namespace: serviceKey.Namespace},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{{Port: dudPort}},
},
}

Expect(k8sClient.Create(ctx, &existingSrviceWithOutOwnerRef)).Should(Succeed())
retrievedService := corev1.Service{}
// ensure it is present before the artemis cr
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, serviceKey, &retrievedService)).Should(Succeed())
g.Expect(retrievedService.ResourceVersion).Should(Not(BeEmpty()))
}, existingClusterTimeout, existingClusterInterval).Should(Succeed())

crd.Spec.Console.Expose = true
crd.Spec.Console.SSLEnabled = true
crd.Spec.Console.SSLSecret = "my-secret"
Expand Down Expand Up @@ -1078,10 +1097,22 @@ var _ = Describe("artemis controller", func() {
}, existingClusterTimeout, existingClusterInterval).Should(Succeed())
}

By("verify service still exists and is updated")
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, serviceKey, &retrievedService)).Should(Succeed())
g.Expect(len(retrievedService.GetOwnerReferences())).Should(BeEquivalentTo(1))
g.Expect(retrievedService.Spec.Ports[0].Port).Should(Not(BeEquivalentTo(dudPort)))
}, existingClusterTimeout, existingClusterInterval).Should(Succeed())

Expect(k8sClient.Delete(ctx, &crd)).Should(Succeed())
if isOpenshift {
Expect(k8sClient.Delete(ctx, &consoleSecret)).Should(Succeed())
}

By("verify service gets owned and deleted")
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, serviceKey, &retrievedService)).Should(Not(Succeed()))
}, existingClusterTimeout, existingClusterInterval).Should(Succeed())
}
})
})
Expand Down
61 changes: 52 additions & 9 deletions controllers/activemqartemis_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,26 @@ func trackSecretCheckSumInEnvVar(requestedResources []rtclient.Object, container
}

func (reconciler *ActiveMQArtemisReconcilerImpl) cloneOfDeployed(kind reflect.Type, name string) rtclient.Object {
obj := reconciler.getFromDeployed(kind, name)
if obj != nil {
return obj.DeepCopyObject().(rtclient.Object)
}
return nil
}

func (reconciler *ActiveMQArtemisReconcilerImpl) getFromDeployed(kind reflect.Type, name string) rtclient.Object {
for _, obj := range reconciler.deployed[kind] {
if obj.GetName() == name {
return obj.DeepCopyObject().(rtclient.Object)
return obj
}
}
return nil
}

func (reconciler *ActiveMQArtemisReconcilerImpl) addToDeployed(kind reflect.Type, obj rtclient.Object) {
reconciler.deployed[kind] = append(reconciler.deployed[kind], obj)
}

func (reconciler *ActiveMQArtemisReconcilerImpl) ProcessStatefulSet(customResource *brokerv1beta1.ActiveMQArtemis, namer Namers, client rtclient.Client, log logr.Logger) (*appsv1.StatefulSet, error) {

reqLogger := ctrl.Log.WithName(customResource.Name)
Expand Down Expand Up @@ -788,15 +800,9 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) configureConsoleExposure(custom

serviceDefinition := svc.NewServiceDefinitionForCR(client, namespacedName, targetPortName, portNumber, serviceRoutelabels, namer.LabelBuilder.Labels())

serviceNamespacedName := types.NamespacedName{
Name: serviceDefinition.Name,
Namespace: customResource.Namespace,
}
if console.Expose {
reconciler.checkExistingService(customResource, serviceDefinition, client)
reconciler.trackDesired(serviceDefinition)
//causedUpdate, err = resources.Enable(customResource, client, scheme, serviceNamespacedName, serviceDefinition)
} else {
causedUpdate, err = resources.Disable(customResource, client, scheme, serviceNamespacedName, serviceDefinition)
}
var err error = nil
isOpenshift := false
Expand Down Expand Up @@ -1238,7 +1244,7 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) deleteResource(customResource *
}

func (reconciler *ActiveMQArtemisReconcilerImpl) createRequestedResource(customResource *brokerv1beta1.ActiveMQArtemis, client rtclient.Client, scheme *runtime.Scheme, requested rtclient.Object, reqLogger logr.Logger, kind reflect.Type) error {
reqLogger.Info("Createing ", "kind ", kind, "named ", requested.GetName())
reqLogger.Info("Creating ", "kind ", kind, "named ", requested.GetName())

return resources.Create(customResource, client, scheme, requested)
}
Expand All @@ -1265,6 +1271,43 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) deleteRequestedResource(customR
return deleteError
}

func (reconciler *ActiveMQArtemisReconcilerImpl) checkExistingService(cr *brokerv1beta1.ActiveMQArtemis, candidate *corev1.Service, client rtclient.Client) {
var log = ctrl.Log.WithName("controller_v1beta1activemqartemis")

serviceType := reflect.TypeOf(corev1.Service{})
obj := reconciler.getFromDeployed(serviceType, candidate.Name)
if obj != nil {
// happy path we already own this
return
}
if obj == nil {
// check for existing match and track
key := types.NamespacedName{Name: candidate.Name, Namespace: candidate.Namespace}
existingService := &corev1.Service{}
err := client.Get(context.TODO(), key, existingService)
if err == nil {
if len(existingService.OwnerReferences) == 0 {
gvk := cr.GroupVersionKind()
existingService.OwnerReferences = []metav1.OwnerReference{{
APIVersion: gvk.GroupVersion().String(),
Kind: gvk.Kind,
Name: cr.GetName(),
UID: cr.GetUID()}}

// we need to have 'candidate' match for update
candidate.ResourceVersion = existingService.ResourceVersion
candidate.UID = existingService.UID
candidate.OwnerReferences = existingService.OwnerReferences
reconciler.addToDeployed(serviceType, existingService)
log.Info("found matching service without owner reference, reclaiming", "Name", key)

} else {
log.Info("found matching service with unexpected owner reference, it may need manual removal", "Name", key)
}
}
}
}

func checkExistingPersistentVolumes(instance *brokerv1beta1.ActiveMQArtemis, client rtclient.Client) {
var log = ctrl.Log.WithName("controller_v1beta1activemqartemis")

Expand Down
49 changes: 0 additions & 49 deletions pkg/resources/k8s_actions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,52 +134,3 @@ func Delete(client client.Client, clientObject client.Object) error {

return err
}

func Enable(owner v1.Object, client client.Client, scheme *runtime.Scheme, namespacedName types.NamespacedName, clientObject client.Object) (bool, error) {
causedUpdate, err := configureExposure(owner, client, scheme, namespacedName, clientObject, true)
return causedUpdate, err
}

func Disable(owner v1.Object, client client.Client, scheme *runtime.Scheme, namespacedName types.NamespacedName, clientObject client.Object) (bool, error) {
causedUpdate, err := configureExposure(owner, client, scheme, namespacedName, clientObject, false)
return causedUpdate, err
}

func configureExposure(owner v1.Object, client client.Client, scheme *runtime.Scheme, namespacedName types.NamespacedName, clientObject client.Object, enable bool) (bool, error) {

// Log where we are and what we're doing
reqLogger := log.WithValues("ActiveMQArtemis Name ", namespacedName.Name)
objectTypeString := reflect.TypeOf(clientObject.(runtime.Object)).String()
reqLogger.Info("Configuring " + objectTypeString)

var err error = nil
serviceIsNotFound := false
causedUpdate := true

if err = Retrieve(namespacedName, client, clientObject); err != nil {
if errors.IsNotFound(err) {
reqLogger.Info(namespacedName.Name + " " + "not found")
serviceIsNotFound = true
} else {
reqLogger.Error(err, "Unexpected error occurred in retrieve", "object def", clientObject)
}
}

// We want a service to be exposed and currently it is not found
if enable && serviceIsNotFound {
reqLogger.Info("Creating " + namespacedName.Name)
if err = Create(owner, client, scheme, clientObject); err != nil {
causedUpdate = true
}
}

// We do NOT want a service to be exposed and the service IS found
if !enable && !serviceIsNotFound {
reqLogger.Info("Deleting " + namespacedName.Name)
if err = Delete(client, clientObject); err != nil {
causedUpdate = true
}
}

return causedUpdate, err
}

0 comments on commit de06e2f

Please sign in to comment.