8000 fix: ignore resource.Quantity string cache in ActionPlan diff by yuminn-k · Pull Request #2130 · Altinity/clickhouse-operator · GitHub
[go: up one dir, main page]
Skip to content

fix: ignore resource.Quantity string cache in ActionPlan diff #2130

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.

Sign up for GitHub

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

Open
yuminn-k wants to merge 3 commits into
base: 0.28.0
Choose a base branch
from
Open

fix: ignore resource.Quantity string cache in ActionPlan diff #2130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ab2016
fix: ignore resource.Quantity string cache in ActionPlan diff
yuminn-k Oct 31, 2026
add8b2a
Merge branch '0.28.0' into fix-action-plan-quantity-string
Slach Nov 3, 2026
abce9bf
Merge branch '0.28.0' into fix-action-plan-quantity-string
Slach Nov 6, 2026
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
10000
5 changes: 5 additions & 0 deletions pkg/apis/clickhouse.altinity.com/v1/action_plan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,11 @@ func (ap *ActionPlan) isExcludedPathSegment(prev, cur string) bool {
return true
}

// resource.Quantity caches its string form lazily in .s; the value itself lives in its other fields
if cur == ".s" {
return true
}

if (((prev == "TaskID") && (cur == ".TaskID")) ||
((prev == ".TaskID") && (cur == ".TaskID"))) && ap.skipTaskID {
return true
Expand Down
36 changes: 36 additions & 0 deletions pkg/apis/clickhouse.altinity.com/v1/action_plan_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"testing"

"github.com/stretchr/testify/require"
core "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
)

// chiWithReplicas builds a CHI whose diff against another has SEVERAL modified entries.
Expand Down Expand Up @@ -182,3 +184,37 @@ func TestActionPlanDeepCopyIsRaceFree(t *testing.T) {
close(start)
wg.Wait()
}

// chiWithMemoryLimit builds a CHI whose pod template limits memory to the given quantity.
func chiWithMemoryLimit(memory resource.Quantity) *ClickHouseInstallation {
cr := &ClickHouseInstallation{}
cr.EnsureRuntime()
cr.Spec.Templates = &Templates{
PodTemplates: []PodTemplate{{
Name: "default",
Spec: core.PodSpec{Containers: []core.Container{{
Name: "clickhouse",
Resources: core.ResourceRequirements{
Limits: core.ResourceList{core.ResourceMemory: memory},
},
}}},
}},
}
return cr
}

// TestActionPlanIgnoresQuantityStringCache pins that equal quantities do not produce a plan.
//
// resource.Quantity caches its string form in an unexported field that is filled lazily, so
// the ancestor and the freshly normalized CR can hold the same 2048Mi with and without the
// cache. Treating that as a spec change re-reconciled the CR forever.
func TestActionPlanIgnoresQuantityStringCache(t *testing.T) {
cached := resource.MustParse("2048Mi")
_ = cached.String()

plan := MakeActionPlan(chiWithMemoryLimit(cached), chiWithMemoryLimit(resource.MustParse("2048Mi")))
require.False(t, plan.HasActionsToDo(), plan.String())

plan = MakeActionPlan(chiWithMemoryLimit(cached), chiWithMemoryLimit(resource.MustParse("4Gi")))
require.True(t, plan.HasActionsToDo(), "a real quantity change must still be planned")
}
0