Skip to content

Commit

Permalink
[#1007] respect deployed resource on PVC reconcile and fill in volume…
Browse files Browse the repository at this point in the history 
…Mode default
  • Loading branch information
@gtully
gtully committed Sep 17, 2024
1 parent 4be587f commit de81619
Show file tree
Hide file tree
Showing 4 changed files with 182 additions and 86 deletions.
96 changes: 95 additions & 1 deletion controllers/activemqartemis_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3241,7 +3241,7 @@ var _ = Describe("artemis controller", func() {
g.Expect(meta.IsStatusConditionTrue(deployedCrd.Status.Conditions, brokerv1beta1.ReadyConditionType)).Should(BeTrue())
}, existingClusterTimeout, existingClusterInterval).Should(Succeed())

unequalEntries, _ := FindAllInCapturingLog("Unequal")
unequalEntries, _ := FindAllInCapturingLog("unequal")
Expect(len(unequalEntries)).Should(BeNumerically("==", 0))

Expect(k8sClient.Delete(ctx, deployedCrd)).Should(Succeed())
Expand Down Expand Up @@ -6729,6 +6729,100 @@ var _ = Describe("artemis controller", func() {
})
})

Context("With update persistence=true single reconcile", func() {
It("check reconcole loop", func() {
By("By creating a crd persistence")
ctx := context.Background()
crd := generateArtemisSpec(defaultNamespace)

crd.Spec.DeploymentPlan.PersistenceEnabled = true
if !isOpenshift {
crd.Spec.IngressDomain = defaultTestIngressDomain
}

Expect(k8sClient.Create(ctx, &crd)).Should(Succeed())

if os.Getenv("USE_EXISTING_CLUSTER") == "true" {

brokerKey := types.NamespacedName{Name: crd.ObjectMeta.Name, Namespace: crd.ObjectMeta.Namespace}
createdCrd := &brokerv1beta1.ActiveMQArtemis{}

By("wait for ready indication")
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, brokerKey, createdCrd)).Should(Succeed())
if verbose {
fmt.Printf("\nSTATUS: %v\n", createdCrd.Status)
}
g.Expect(meta.IsStatusConditionTrue(createdCrd.Status.Conditions, brokerv1beta1.DeployedConditionType)).Should(BeTrue())
g.Expect(meta.IsStatusConditionTrue(createdCrd.Status.Conditions, brokerv1beta1.ReadyConditionType)).Should(BeTrue())

}, existingClusterTimeout, existingClusterInterval).Should(Succeed())

createdSs := &appsv1.StatefulSet{}
By("finding ss resource version")
Eventually(func(g Gomega) {
key := types.NamespacedName{Name: namer.CrToSS(crd.Name), Namespace: defaultNamespace}
g.Expect(k8sClient.Get(ctx, key, createdSs)).Should(Succeed())
g.Expect(createdSs.ObjectMeta.ResourceVersion).NotTo(Equal(""))
}, timeout, interval).Should(Succeed())

initialVersion := createdSs.ObjectMeta.ResourceVersion

if os.Getenv("DEPLOY_OPERATOR") == "false" {

By("start to capture test log needs local operator")
StartCapturingLog()
defer StopCapturingLog()

By("updating the crd to expose console - verify no ss mod")
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, brokerKey, createdCrd)).Should(Succeed())
createdCrd.Spec.Console.Expose = true
createdCrd.Spec.Console.ExposeMode = &brokerv1beta1.ExposeModes.Ingress
g.Expect(k8sClient.Update(ctx, createdCrd)).Should(Succeed())
}, timeout, interval).Should(Succeed())

By("check ingress is created for console")
ingKey := types.NamespacedName{
Name: createdCrd.Name + "-wconsj-0-svc-ing",
Namespace: defaultNamespace,
}
ingress := netv1.Ingress{}
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, ingKey, &ingress)).To(Succeed())
}, existingClusterTimeout, existingClusterInterval).Should(Succeed())

By("wait for ready indication")
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, brokerKey, createdCrd)).Should(Succeed())
if verbose {
fmt.Printf("\nSTATUS: %v\n", createdCrd.Status)
}
g.Expect(meta.IsStatusConditionTrue(createdCrd.Status.Conditions, brokerv1beta1.DeployedConditionType)).Should(BeTrue())
g.Expect(meta.IsStatusConditionTrue(createdCrd.Status.Conditions, brokerv1beta1.ReadyConditionType)).Should(BeTrue())

}, existingClusterTimeout, existingClusterInterval).Should(Succeed())

By("Making sure that the ss does not get redeployed " + crd.ObjectMeta.Name)
Eventually(func(g Gomega) {
key := types.NamespacedName{Name: namer.CrToSS(crd.Name), Namespace: defaultNamespace}
g.Expect(k8sClient.Get(ctx, key, createdSs)).Should(Succeed())
g.Expect(initialVersion).To(Equal(createdSs.ObjectMeta.ResourceVersion))
}, timeout, interval).Should(Succeed())

By("finding no Updating v1.StatefulSet ")
matches, err := FindAllInCapturingLog(`Updating \*v1.StatefulSet`)
Expect(err).To(BeNil())
Expect(len(matches)).To(Equal(0))
}

}

// cleanup
CleanResource(&crd, crd.Name, defaultNamespace)
})
})

Context("Toggle spec.Version", func() {
It("Expect ok update and new SS generation", func() {

Expand Down
93 changes: 57 additions & 36 deletions controllers/activemqartemis_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"k8s.io/apimachinery/pkg/api/equality"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -265,7 +266,7 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) ProcessStatefulSet(customResour
}

reqLogger.V(2).Info("Reconciling desired statefulset", "name", ssNamespacedName, "current", currentStatefulSet)
currentStatefulSet, err = reconciler.NewStatefulSetForCR(customResource, namer, currentStatefulSet, client)
currentStatefulSet, err = reconciler.StatefulSetForCR(customResource, namer, currentStatefulSet, client)
if err != nil {
reqLogger.Error(err, "Error creating new stafulset")
return nil, err
Expand Down Expand Up @@ -1497,7 +1498,7 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) CompareMetaAndSpec(deployed, re
isEqual := equalObjectMeta(deployed, requested) &&
equality.Semantic.DeepEqual(specOf(deployed), specOf(requested))
if !isEqual {
reconciler.log.V(2).Info("unequal", "deployed", deployed, "requested", requested)
reconciler.log.V(2).Info("unequal", "deployed", &deployed, "requested", &requested)
}
return isEqual
}
Expand Down Expand Up @@ -1894,7 +1895,7 @@ func MakeContainerPorts(cr *brokerv1beta1.ActiveMQArtemis) []corev1.ContainerPor
return containerPorts
}

func (reconciler *ActiveMQArtemisReconcilerImpl) NewPodTemplateSpecForCR(customResource *brokerv1beta1.ActiveMQArtemis, namer common.Namers, current *corev1.PodTemplateSpec, client rtclient.Client) (*corev1.PodTemplateSpec, error) {
func (reconciler *ActiveMQArtemisReconcilerImpl) PodTemplateSpecForCR(customResource *brokerv1beta1.ActiveMQArtemis, namer common.Namers, current *corev1.PodTemplateSpec, client rtclient.Client) (*corev1.PodTemplateSpec, error) {

reqLogger := reconciler.log.WithName(customResource.Name)

Expand Down Expand Up @@ -2293,7 +2294,7 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) configureStartupProbe(container
startupProbe = &corev1.Probe{}
}

applyNonDefaultedValues(startupProbe, probeFromCr)
conditionallyApplyValuesToPreserveDefaults(startupProbe, probeFromCr)
startupProbe.ProbeHandler = probeFromCr.ProbeHandler
} else {
startupProbe = nil
Expand All @@ -2311,7 +2312,7 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) configureLivenessProbe(containe
}

if probeFromCr != nil {
applyNonDefaultedValues(livenessProbe, probeFromCr)
conditionallyApplyValuesToPreserveDefaults(livenessProbe, probeFromCr)

// not complete in this case!
if probeFromCr.GRPC == nil && probeFromCr.Exec == nil && probeFromCr.HTTPGet == nil && probeFromCr.TCPSocket == nil {
Expand Down Expand Up @@ -2363,7 +2364,7 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) configureReadinessProbe(contain
}

if probeFromCr != nil {
applyNonDefaultedValues(readinessProbe, probeFromCr)
conditionallyApplyValuesToPreserveDefaults(readinessProbe, probeFromCr)
if probeFromCr.GRPC == nil && probeFromCr.Exec == nil && probeFromCr.HTTPGet == nil && probeFromCr.TCPSocket == nil {
reconciler.log.V(2).Info("adding default handler to user provided readiness Probe")

Expand Down Expand Up @@ -2402,8 +2403,9 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) configureReadinessProbe(contain
return readinessProbe
}

func applyNonDefaultedValues(readinessProbe *corev1.Probe, probeFromCr *corev1.Probe) {
//copy the probe, but only for non default (init values)
// when the CR has a full Spec, the intent is that the Spec is fully formed, such that there are not server side defaults in the mix.
// For probes, we historically allow a partial spec, so we need to be careful to not overide server side applied defaults with empty values
func conditionallyApplyValuesToPreserveDefaults(readinessProbe *corev1.Probe, probeFromCr *corev1.Probe) {
if probeFromCr.InitialDelaySeconds > 0 {
readinessProbe.InitialDelaySeconds = probeFromCr.InitialDelaySeconds
}
Expand Down Expand Up @@ -2705,7 +2707,7 @@ func ParseBrokerPropertyWithOrdinal(property string) []string {
return brokerPropertyWithOrdinalRegex.FindStringSubmatch(property)
}

func (reconciler *ActiveMQArtemisReconcilerImpl) NewStatefulSetForCR(customResource *brokerv1beta1.ActiveMQArtemis, namer common.Namers, currentStateFullSet *appsv1.StatefulSet, client rtclient.Client) (*appsv1.StatefulSet, error) {
func (reconciler *ActiveMQArtemisReconcilerImpl) StatefulSetForCR(customResource *brokerv1beta1.ActiveMQArtemis, namer common.Namers, currentStateFullSet *appsv1.StatefulSet, client rtclient.Client) (*appsv1.StatefulSet, error) {

reqLogger := reconciler.log.WithName(customResource.Name)

Expand All @@ -2716,56 +2718,75 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) NewStatefulSetForCR(customResou
replicas := common.GetDeploymentSize(customResource)
currentStateFullSet = ss.MakeStatefulSet(currentStateFullSet, namer.SsNameBuilder.Name(), namer.SvcHeadlessNameBuilder.Name(), namespacedName, nil, namer.LabelBuilder.Labels(), &replicas)

podTemplateSpec, err := reconciler.NewPodTemplateSpecForCR(customResource, namer, &currentStateFullSet.Spec.Template, client)
podTemplateSpec, err := reconciler.PodTemplateSpecForCR(customResource, namer, &currentStateFullSet.Spec.Template, client)
if err != nil {
reqLogger.Error(err, "Error creating new pod template")
reqLogger.Error(err, "error creating pod template")
return nil, err
}

if customResource.Spec.DeploymentPlan.PersistenceEnabled || len(customResource.Spec.DeploymentPlan.ExtraVolumeClaimTemplates) > 0 {
currentStateFullSet.Spec.VolumeClaimTemplates = *reconciler.NewPersistentVolumeClaimArrayForCR(customResource, namer, 1)
}
currentStateFullSet.Spec.Template = *podTemplateSpec

currentStateFullSet.Spec.VolumeClaimTemplates = reconciler.PersistentVolumeClaimArrayForCR(customResource, namer, currentStateFullSet.Spec)

return currentStateFullSet, nil
}

func (reconciler *ActiveMQArtemisReconcilerImpl) NewPersistentVolumeClaimArrayForCR(customResource *brokerv1beta1.ActiveMQArtemis, namer common.Namers, arrayLength int) *[]corev1.PersistentVolumeClaim {
func (reconciler *ActiveMQArtemisReconcilerImpl) PersistentVolumeClaimArrayForCR(customResource *brokerv1beta1.ActiveMQArtemis, namer common.Namers, spec appsv1.StatefulSetSpec) []corev1.PersistentVolumeClaim {

var pvc *corev1.PersistentVolumeClaim = nil

capacity := "2Gi"
pvcArray := make([]corev1.PersistentVolumeClaim, 0, arrayLength)
storageClassName := ""
var existing, current *corev1.PersistentVolumeClaim
pvcArray := make([]corev1.PersistentVolumeClaim, 0)

if customResource.Spec.DeploymentPlan.PersistenceEnabled {

namespacedName := types.NamespacedName{
Name: customResource.Name,
Namespace: customResource.Namespace,
}

capacity := "2Gi"
if customResource.Spec.DeploymentPlan.Storage.Size != "" {
capacity = customResource.Spec.DeploymentPlan.Storage.Size
}

tempateClaim := &brokerv1beta1.VolumeClaimTemplate{
ObjectMeta: brokerv1beta1.ObjectMeta{
Name: customResource.Name,
Labels: namer.LabelBuilder.Labels(),
},
Spec: corev1.PersistentVolumeClaimSpec{
AccessModes: []corev1.PersistentVolumeAccessMode{"ReadWriteOnce"},
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceName(corev1.ResourceStorage): resource.MustParse(capacity),
},
},
},
}
if customResource.Spec.DeploymentPlan.Storage.StorageClassName != "" {
storageClassName = customResource.Spec.DeploymentPlan.Storage.StorageClassName
tempateClaim.Spec.StorageClassName = &customResource.Spec.DeploymentPlan.Storage.StorageClassName
}

for i := 0; i < arrayLength; i++ {
pvc = persistentvolumeclaims.NewPersistentVolumeClaimWithCapacityAndStorageClassName(namespacedName, capacity, namer.LabelBuilder.Labels(), storageClassName, []corev1.PersistentVolumeAccessMode{"ReadWriteOnce"})
reconciler.applyTemplates(pvc)
pvcArray = append(pvcArray, *pvc)
}
existing = findExistingByName(spec.VolumeClaimTemplates, tempateClaim)
current = persistentvolumeclaims.PersistentVolumeClaim(customResource.Namespace, existing, tempateClaim)
pvcArray = append(pvcArray, *current)
}

for _, epvc := range customResource.Spec.DeploymentPlan.ExtraVolumeClaimTemplates {
pvc = persistentvolumeclaims.NewPersistentVolumeClaim(customResource.Namespace, &epvc)
pvcArray = append(pvcArray, *pvc)
existing = findExistingByName(spec.VolumeClaimTemplates, &epvc)
current = persistentvolumeclaims.PersistentVolumeClaim(customResource.Namespace, existing, &epvc)
pvcArray = append(pvcArray, *current)
}

for index := range pvcArray {
reconciler.applyTemplates(&pvcArray[index])
}

return &pvcArray
if len(pvcArray) > 0 {
return pvcArray
}
return nil
}

func findExistingByName(persistentVolumeClaim []corev1.PersistentVolumeClaim, tempateClaim *brokerv1beta1.VolumeClaimTemplate) *corev1.PersistentVolumeClaim {
for index, claim := range persistentVolumeClaim {
if claim.Name == tempateClaim.Name {
return &persistentVolumeClaim[index]
}
}
return nil
}

func MakeEnvVarArrayForCR(customResource *brokerv1beta1.ActiveMQArtemis, namer common.Namers) []corev1.EnvVar {
Expand Down
10 changes: 5 additions & 5 deletions controllers/activemqartemis_reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ func TestNewPodTemplateSpecForCR_IncludesDebugArgs(t *testing.T) {
customResource: cr,
}

newSpec, err := reconciler.NewPodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)
newSpec, err := reconciler.PodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)

assert.NoError(t, err)
assert.NotNil(t, newSpec)
Expand Down Expand Up @@ -1057,7 +1057,7 @@ func TestNewPodTemplateSpecForCR_AppendsDebugArgs(t *testing.T) {
outer := NewActiveMQArtemisReconciler(&NillCluster{}, ctrl.Log.WithName("test"), isOpenshift)
reconciler := NewActiveMQArtemisReconcilerImpl(cr, outer)

newSpec, err := reconciler.NewPodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)
newSpec, err := reconciler.PodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)

assert.NoError(t, err)
assert.NotNil(t, newSpec)
Expand All @@ -1084,7 +1084,7 @@ func TestNewPodTemplateSpecForCR_IncludesImagePullSecret(t *testing.T) {
outer := NewActiveMQArtemisReconciler(&NillCluster{}, ctrl.Log, isOpenshift)
reconciler := NewActiveMQArtemisReconcilerImpl(cr, outer)

newSpec, err := reconciler.NewPodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)
newSpec, err := reconciler.PodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)
assert.NoError(t, err)
assert.NotNil(t, newSpec)
expectedPullSecret := []v1.LocalObjectReference{
Expand Down Expand Up @@ -1120,7 +1120,7 @@ func TestNewPodTemplateSpecForCR_IncludesTopologySpreadConstraints(t *testing.T)
outer := NewActiveMQArtemisReconciler(&NillCluster{}, ctrl.Log, isOpenshift)
reconciler := NewActiveMQArtemisReconcilerImpl(cr, outer)

newSpec, err := reconciler.NewPodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)
newSpec, err := reconciler.PodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)
assert.NoError(t, err)
assert.NotNil(t, newSpec)
expectedTopologySpreadConstraints := []v1.TopologySpreadConstraint{
Expand Down Expand Up @@ -1150,7 +1150,7 @@ func TestNewPodTemplateSpecForCR_IncludesContainerSecurityContext(t *testing.T)
customResource: cr,
}

newSpec, err := reconciler.NewPodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)
newSpec, err := reconciler.PodTemplateSpecForCR(cr, common.Namers{}, &v1.PodTemplateSpec{}, k8sClient)

assert.NoError(t, err)
assert.NotNil(t, newSpec)
Expand Down
Loading

0 comments on commit de81619

Please sign in to comment.