Skip to content

Commit

Permalink
[release-1.8] Introduce a new jsonpatch annotation for SSP (#2229) (#…
Browse files Browse the repository at this point in the history 
…2240)

Introduce an additional jsonpatch annotation
to let the cluster admin override also SSP
configuration for test/dev purposes.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2166923
JIRA-ticket: https://issues.redhat.com/browse/CNV-25035

This is a manual cherry-pick of #2229

Signed-off-by: Simone Tiraboschi <[email protected]>
  • Loading branch information
@tiraboschi
tiraboschi authored Feb 10, 2023
1 parent 2cbbad3 commit c59af1f
Show file tree
Hide file tree
Showing 9 changed files with 390 additions and 3 deletions.
1 change: 1 addition & 0 deletions controllers/common/consts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ const (
JSONPatchKVAnnotationName = "kubevirt.kubevirt.io/jsonpatch"
JSONPatchCDIAnnotationName = "containerizeddataimporter.kubevirt.io/jsonpatch"
JSONPatchCNAOAnnotationName = "networkaddonsconfigs.kubevirt.io/jsonpatch"
JSONPatchSSPAnnotationName = "ssp.kubevirt.io/jsonpatch"
)
1 change: 1 addition & 0 deletions controllers/hyperconverged/hyperconverged_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ var JSONPatchAnnotationNames = []string{
common.JSONPatchKVAnnotationName,
common.JSONPatchCDIAnnotationName,
common.JSONPatchCNAOAnnotationName,
common.JSONPatchSSPAnnotationName,
}

// RegisterReconciler creates a new HyperConverged Reconciler and registers it into manager.
Expand Down
156 changes: 156 additions & 0 deletions controllers/hyperconverged/hyperconverged_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3557,6 +3557,141 @@ var _ = Describe("HyperconvergedController", func() {
})
})

Context("Detection of a tainted configuration for SSP", func() {

It("Raises a TaintedConfiguration condition upon detection of such configuration", func() {
hco.ObjectMeta.Annotations = map[string]string{
common.JSONPatchSSPAnnotationName: `[
{
"op": "replace",
"path": "/spec/templateValidator/replicas",
"value": 5
}
]`,
}

err := metrics.HcoMetrics.SetUnsafeModificationCount(0, common.JSONPatchSSPAnnotationName)
Expect(err).ToNot(HaveOccurred())

cl := commonTestUtils.InitClient([]runtime.Object{hcoNamespace, hco})
r := initReconciler(cl, nil)

By("Reconcile", func() {
res, err := r.Reconcile(context.TODO(), request)
Expect(err).ToNot(HaveOccurred())
Expect(res).Should(Equal(reconcile.Result{Requeue: true}))
})

foundResource := &hcov1beta1.HyperConverged{}
Expect(
cl.Get(context.TODO(),
types.NamespacedName{Name: hco.Name, Namespace: hco.Namespace},
foundResource),
).To(Succeed())

By("Verify HC conditions", func() {
Expect(foundResource.Status.Conditions).To(ContainElement(commonTestUtils.RepresentCondition(metav1.Condition{
Type: hcov1beta1.ConditionTaintedConfiguration,
Status: metav1.ConditionTrue,
Reason: taintedConfigurationReason,
Message: taintedConfigurationMessage,
})))
})

By("verify that the metrics match to the annotation", func() {
verifyUnsafeMetrics(1, common.JSONPatchSSPAnnotationName)
})

By("Verify that SSP was modified by the annotation", func() {
ssp := operands.NewSSPWithNameOnly(hco)
Expect(
cl.Get(context.TODO(),
types.NamespacedName{Name: ssp.Name, Namespace: ssp.Namespace},
ssp),
).To(Succeed())

Expect(ssp.Spec.TemplateValidator.Replicas).Should(Not(BeNil()))
Expect(*ssp.Spec.TemplateValidator.Replicas).Should(Equal(int32(5)))
})
})

It("Removes the TaintedConfiguration condition upon removal of such configuration", func() {
hco.Status.Conditions = append(hco.Status.Conditions, metav1.Condition{
Type: hcov1beta1.ConditionTaintedConfiguration,
Status: metav1.ConditionTrue,
Reason: taintedConfigurationReason,
Message: taintedConfigurationMessage,
})
err := metrics.HcoMetrics.SetUnsafeModificationCount(5, common.JSONPatchSSPAnnotationName)
Expect(err).ToNot(HaveOccurred())

cl := commonTestUtils.InitClient([]runtime.Object{hcoNamespace, hco})
r := initReconciler(cl, nil)

// Do the reconcile
res, err := r.Reconcile(context.TODO(), request)
Expect(err).ToNot(HaveOccurred())

// Expecting "Requeue: false" since the conditions aren't empty
Expect(res).Should(Equal(reconcile.Result{Requeue: true}))

// Get the HCO
foundResource := &hcov1beta1.HyperConverged{}
Expect(
cl.Get(context.TODO(),
types.NamespacedName{Name: hco.Name, Namespace: hco.Namespace},
foundResource),
).To(Succeed())

// Check conditions
Expect(foundResource.Status.Conditions).To(Not(ContainElement(commonTestUtils.RepresentCondition(metav1.Condition{
Type: hcov1beta1.ConditionTaintedConfiguration,
Status: metav1.ConditionTrue,
Reason: taintedConfigurationReason,
Message: taintedConfigurationMessage,
}))))
By("verify that the metrics match to the annotation", func() {
verifyUnsafeMetrics(0, common.JSONPatchSSPAnnotationName)
})
})

It("Removes the TaintedConfiguration condition if the annotation is wrong", func() {
hco.ObjectMeta.Annotations = map[string]string{
// Set bad json
common.JSONPatchSSPAnnotationName: `[{`,
}
err := metrics.HcoMetrics.SetUnsafeModificationCount(5, common.JSONPatchSSPAnnotationName)
Expect(err).ToNot(HaveOccurred())

cl := commonTestUtils.InitClient([]runtime.Object{hcoNamespace, hco})
r := initReconciler(cl, nil)

By("Reconcile", func() {
res, err := r.Reconcile(context.TODO(), request)
Expect(err).ToNot(HaveOccurred())
Expect(res).Should(Equal(reconcile.Result{Requeue: true}))
})

foundResource := &hcov1beta1.HyperConverged{}
Expect(
cl.Get(context.TODO(),
types.NamespacedName{Name: hco.Name, Namespace: hco.Namespace},
foundResource),
).To(Succeed())

// Check conditions
Expect(foundResource.Status.Conditions).To(Not(ContainElement(commonTestUtils.RepresentCondition(metav1.Condition{
Type: hcov1beta1.ConditionTaintedConfiguration,
Status: metav1.ConditionTrue,
Reason: taintedConfigurationReason,
Message: taintedConfigurationMessage,
}))))
By("verify that the metrics match to the annotation", func() {
verifyUnsafeMetrics(0, common.JSONPatchSSPAnnotationName)
})
})
})

Context("Detection of a tainted configuration for all the annotations", func() {
It("Raises a TaintedConfiguration condition upon detection of such configuration", func() {
hco.ObjectMeta.Annotations = map[string]string{
Expand Down Expand Up @@ -3592,13 +3727,22 @@ var _ = Describe("HyperconvergedController", func() {
"value": "Always"
}
]`,
common.JSONPatchSSPAnnotationName: `[
{
"op": "replace",
"path": "/spec/templateValidator/replicas",
"value": 5
}
]`,
}
err := metrics.HcoMetrics.SetUnsafeModificationCount(0, common.JSONPatchKVAnnotationName)
Expect(err).ToNot(HaveOccurred())
err = metrics.HcoMetrics.SetUnsafeModificationCount(0, common.JSONPatchCDIAnnotationName)
Expect(err).ToNot(HaveOccurred())
err = metrics.HcoMetrics.SetUnsafeModificationCount(0, common.JSONPatchCNAOAnnotationName)
Expect(err).ToNot(HaveOccurred())
err = metrics.HcoMetrics.SetUnsafeModificationCount(0, common.JSONPatchSSPAnnotationName)
Expect(err).ToNot(HaveOccurred())

cl := commonTestUtils.InitClient([]runtime.Object{hcoNamespace, hco})
r := initReconciler(cl, nil)
Expand Down Expand Up @@ -3629,6 +3773,7 @@ var _ = Describe("HyperconvergedController", func() {
verifyUnsafeMetrics(1, common.JSONPatchKVAnnotationName)
verifyUnsafeMetrics(2, common.JSONPatchCDIAnnotationName)
verifyUnsafeMetrics(2, common.JSONPatchCNAOAnnotationName)
verifyUnsafeMetrics(1, common.JSONPatchSSPAnnotationName)
})

By("Verify that KV was modified by the annotation", func() {
Expand Down Expand Up @@ -3672,6 +3817,17 @@ var _ = Describe("HyperconvergedController", func() {
Expect(cna.Spec.KubeMacPool.RangeEnd).Should(Equal("5.5.5.5.5.5"))
Expect(cna.Spec.ImagePullPolicy).Should(BeEquivalentTo("Always"))
})
By("Verify that SSP was modified by the annotation", func() {
ssp := operands.NewSSPWithNameOnly(hco)
Expect(
cl.Get(context.TODO(),
types.NamespacedName{Name: ssp.Name, Namespace: ssp.Namespace},
ssp),
).To(Succeed())

Expect(ssp.Spec.TemplateValidator.Replicas).Should(Not(BeNil()))
Expect(*ssp.Spec.TemplateValidator.Replicas).Should(Equal(int32(5)))
})
})
})
})
Expand Down
4 changes: 4 additions & 0 deletions controllers/operands/ssp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ func NewSSP(hc *hcov1beta1.HyperConverged, _ ...string) (*sspv1beta1.SSP, []hcov
ssp := NewSSPWithNameOnly(hc)
ssp.Spec = spec

if err := applyPatchToSpec(hc, common.JSONPatchSSPAnnotationName, ssp); err != nil {
return nil, nil, err
}

return ssp, dataImportCronStatuses, nil
}

Expand Down
153 changes: 151 additions & 2 deletions controllers/operands/ssp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ import (

openshiftconfigv1 "github.com/openshift/api/config/v1"

"k8s.io/utils/pointer"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/reference"
"k8s.io/utils/pointer"

hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/api/v1beta1"
"github.com/kubevirt/hyperconverged-cluster-operator/controllers/common"
Expand Down Expand Up @@ -297,6 +298,154 @@ var _ = Describe("SSP Operands", func() {
})
})

Context("jsonpath Annotation", func() {
It("Should create SSP object with changes from the annotation", func() {
hco.Annotations = map[string]string{common.JSONPatchSSPAnnotationName: `[
{
"op": "replace",
"path": "/spec/templateValidator/replicas",
"value": 5
}
]`}

ssp, _, err := NewSSP(hco)
Expect(err).ToNot(HaveOccurred())
Expect(ssp).ToNot(BeNil())
Expect(ssp.Spec.TemplateValidator.Replicas).Should(Not(BeNil()))
Expect(*ssp.Spec.TemplateValidator.Replicas).Should(Equal(int32(5)))
})

It("Should fail to create SSP object with wrong jsonPatch", func() {
hco.Annotations = map[string]string{common.JSONPatchSSPAnnotationName: `[
{
"op": "notExists",
"path": "/spec/templateValidator/replicas",
"value": 5
}
]`}

_, _, err := NewSSP(hco)
Expect(err).To(HaveOccurred())
})

It("Ensure func should create SSP object with changes from the annotation", func() {
hco.Annotations = map[string]string{common.JSONPatchSSPAnnotationName: `[
{
"op": "replace",
"path": "/spec/templateValidator/replicas",
"value": 5
}
]`}

expectedResource := NewSSPWithNameOnly(hco)
cl := commonTestUtils.InitClient([]runtime.Object{})
handler := (*genericOperand)(newSspHandler(cl, commonTestUtils.GetScheme()))
res := handler.ensure(req)
Expect(res.UpgradeDone).To(BeFalse())
Expect(res.Err).ToNot(HaveOccurred())

ssp := &sspv1beta1.SSP{}
Expect(
cl.Get(context.TODO(),
types.NamespacedName{Name: expectedResource.Name, Namespace: expectedResource.Namespace},
ssp),
).To(Succeed())

Expect(ssp).ToNot(BeNil())
Expect(ssp.Spec.TemplateValidator.Replicas).Should(Not(BeNil()))
Expect(*ssp.Spec.TemplateValidator.Replicas).Should(Equal(int32(5)))
})

It("Ensure func should fail to create SSP object with wrong jsonPatch", func() {
hco.Annotations = map[string]string{common.JSONPatchSSPAnnotationName: `[
{
"op": "notExists",
"path": "/spec/templateValidator/replicas",
"value": 5
}
]`}

expectedResource := NewSSPWithNameOnly(hco)
cl := commonTestUtils.InitClient([]runtime.Object{})
handler := (*genericOperand)(newSspHandler(cl, commonTestUtils.GetScheme()))
res := handler.ensure(req)
Expect(res.Err).To(HaveOccurred())

ssp := &sspv1beta1.SSP{}

err := cl.Get(context.TODO(),
types.NamespacedName{Name: expectedResource.Name, Namespace: expectedResource.Namespace},
ssp)

Expect(err).To(HaveOccurred())
Expect(errors.IsNotFound(err)).To(BeTrue())
})

It("Ensure func should update SSP object with changes from the annotation", func() {
existsSsp, _, err := NewSSP(hco)
Expect(err).ToNot(HaveOccurred())

hco.Annotations = map[string]string{common.JSONPatchSSPAnnotationName: `[
{
"op": "replace",
"path": "/spec/templateValidator/replicas",
"value": 5
}
]`}

cl := commonTestUtils.InitClient([]runtime.Object{hco, existsSsp})

handler := (*genericOperand)(newSspHandler(cl, commonTestUtils.GetScheme()))
res := handler.ensure(req)
Expect(res.Err).ToNot(HaveOccurred())
Expect(res.Updated).To(BeTrue())
Expect(res.UpgradeDone).To(BeFalse())

ssp := &sspv1beta1.SSP{}

expectedResource := NewSSPWithNameOnly(hco)
Expect(
cl.Get(context.TODO(),
types.NamespacedName{Name: expectedResource.Name, Namespace: expectedResource.Namespace},
ssp),
).To(Succeed())

Expect(ssp.Spec.TemplateValidator.Replicas).Should(Not(BeNil()))
Expect(*ssp.Spec.TemplateValidator.Replicas).Should(Equal(int32(5)))
})

It("Ensure func should fail to update SSP object with wrong jsonPatch", func() {
existsSsp, _, err := NewSSP(hco)
Expect(err).ToNot(HaveOccurred())

hco.Annotations = map[string]string{common.JSONPatchSSPAnnotationName: `[
{
"op": "notExists",
"path": "/spec/templateValidator/replicas",
"value": 5
}
]`}

cl := commonTestUtils.InitClient([]runtime.Object{hco, existsSsp})

handler := (*genericOperand)(newSspHandler(cl, commonTestUtils.GetScheme()))
res := handler.ensure(req)
Expect(res.Err).To(HaveOccurred())

ssp := &sspv1beta1.SSP{}

expectedResource := NewSSPWithNameOnly(hco)
Expect(
cl.Get(context.TODO(),
types.NamespacedName{Name: expectedResource.Name, Namespace: expectedResource.Namespace},
ssp),
).To(Succeed())

Expect(ssp.Spec.TemplateValidator.Replicas).Should(Not(BeNil()))
Expect(*ssp.Spec.TemplateValidator.Replicas).Should(Equal(int32(defaultTemplateValidatorReplicas)))
})
})

Context("Cache", func() {
cl := commonTestUtils.InitClient([]runtime.Object{})
handler := (*genericOperand)(newSspHandler(cl, commonTestUtils.GetScheme()))
Expand Down
Loading

0 comments on commit c59af1f

Please sign in to comment.