Skip to content

Commit

Permalink
Merge pull request #960 from akrejcir/fix-template-metric-0.19
Browse files Browse the repository at this point in the history
[release-v0.19] fix: Increase reconcile metric only if SSP CR was not changed
  • Loading branch information
kubevirt-bot authored Apr 16, 2024
2 parents df3777a + fbbcbca commit 59c69d3
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 22 deletions.
26 changes: 15 additions & 11 deletions controllers/ssp_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,18 +148,20 @@ func (r *sspReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ct
// Error reading the object - requeue the request.
return ctrl.Result{}, err
}
r.clearCacheIfNeeded(instance)

sspChanged := r.clearCacheIfNeeded(instance)

sspRequest := &common.Request{
Request: req,
Client: r.client,
UncachedReader: r.uncachedReader,
Context: ctx,
Instance: instance,
Logger: reqLogger,
VersionCache: r.subresourceCache,
TopologyMode: r.topologyMode,
CrdList: r.crdList,
Request: req,
Client: r.client,
UncachedReader: r.uncachedReader,
Context: ctx,
Instance: instance,
InstanceChanged: sspChanged,
Logger: reqLogger,
VersionCache: r.subresourceCache,
TopologyMode: r.topologyMode,
CrdList: r.crdList,
}

if !isInitialized(sspRequest.Instance) {
Expand Down Expand Up @@ -230,11 +232,13 @@ func (r *sspReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ct
return ctrl.Result{}, nil
}

func (r *sspReconciler) clearCacheIfNeeded(sspObj *ssp.SSP) {
func (r *sspReconciler) clearCacheIfNeeded(sspObj *ssp.SSP) bool {
if !reflect.DeepEqual(r.lastSspSpec, sspObj.Spec) {
r.subresourceCache = common.VersionCache{}
r.lastSspSpec = sspObj.Spec
return true
}
return false
}

func (r *sspReconciler) clearCache() {
Expand Down
15 changes: 8 additions & 7 deletions internal/common/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ import (

type Request struct {
reconcile.Request
Client client.Client
UncachedReader client.Reader
Context context.Context
Instance *ssp.SSP
Logger logr.Logger
VersionCache VersionCache
TopologyMode osconfv1.TopologyMode
Client client.Client
UncachedReader client.Reader
Context context.Context
Instance *ssp.SSP
InstanceChanged bool
Logger logr.Logger
VersionCache VersionCache
TopologyMode osconfv1.TopologyMode

CrdList crd_watch.CrdList
}
Expand Down
4 changes: 2 additions & 2 deletions internal/operands/common-templates/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (c *commonTemplates) Reconcile(request *common.Request) ([]common.Reconcile
return nil, err
}

if !isUpgradingNow(request) {
if !operatorIsUpgrading(request) && !request.InstanceChanged {
incrementTemplatesRestoredMetric(reconcileTemplatesResults, request.Logger)
}

Expand All @@ -97,7 +97,7 @@ func (c *commonTemplates) Reconcile(request *common.Request) ([]common.Reconcile
return append(reconcileTemplatesResults, oldTemplatesResults...), nil
}

func isUpgradingNow(request *common.Request) bool {
func operatorIsUpgrading(request *common.Request) bool {
return request.Instance.Status.ObservedVersion != common.GetOperatorVersion()
}

Expand Down
29 changes: 27 additions & 2 deletions internal/operands/common-templates/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ var _ = Describe("Common-Templates operand", func() {
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: map[string]string{
common.AppKubernetesPartOfLabel: "template-unit-tests",
common.AppKubernetesVersionLabel: "v1.0.0",
},
},
Spec: ssp.SSPSpec{
CommonTemplates: ssp.CommonTemplates{
Expand All @@ -85,8 +89,9 @@ var _ = Describe("Common-Templates operand", func() {
},
},
},
Logger: log,
VersionCache: common.VersionCache{},
InstanceChanged: false,
Logger: log,
VersionCache: common.VersionCache{},
}
})

Expand Down Expand Up @@ -326,6 +331,26 @@ var _ = Describe("Common-Templates operand", func() {
Expect(desc).To(ContainSubstring("kubevirt_ssp_common_templates_restored_total"))
Expect(value).To(Equal(initialMetricValue))
})

It("should not increase when SSP CR is changed", func() {
const updatedPartOf = "updated-part-of"
const updatedVersion = "v2.0.0"

request.Instance.Labels[common.AppKubernetesPartOfLabel] = updatedPartOf
request.Instance.Labels[common.AppKubernetesVersionLabel] = updatedVersion
request.InstanceChanged = true

_, err := operand.Reconcile(&request)
Expect(err).ToNot(HaveOccurred())

updatedTemplate := getTemplate(request, template)
Expect(updatedTemplate.Labels).To(HaveKeyWithValue(common.AppKubernetesPartOfLabel, updatedPartOf))
Expect(updatedTemplate.Labels).To(HaveKeyWithValue(common.AppKubernetesVersionLabel, updatedVersion))

desc, value := getCommonTemplatesRestoredMetric()
Expect(desc).To(ContainSubstring("kubevirt_ssp_common_templates_restored_total"))
Expect(value).To(Equal(initialMetricValue))
})
})
})

Expand Down

0 comments on commit 59c69d3

Please sign in to comment.