Skip to content

Commit a172e1f

Browse files
SergKSergK
committed
refactor: Use Status().Patch() for git status updates in chain handlers (#227)
Replace Status().Update() with Status().Patch() to reduce conflicts during sequential git status updates in chain handlers. Patch uses server-side merge semantics, only conflicting when the same field is modified concurrently. Changes: - Add updateGitStatusWithPatch helper function with idempotency check - Update PutProject, PutGitLabCIConfig, PutDeployConfigs handlers - Add comprehensive unit tests for sequential update scenarios Signed-off-by: Sergiy Kulanov <[email protected]>
1 parent 718ff0b commit a172e1f

File tree

5 files changed

+287
-12
lines changed

5 files changed

+287
-12
lines changed

‎controllers/codebase/service/chain/common.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,34 @@ func setIntermediateSuccessFields(ctx context.Context, c client.Client, cb *code
4040

4141
return nil
4242
}
43+
44+
// updateGitStatusWithPatch updates the codebase Git status using Patch instead of Update.
45+
// If a conflict occurs, the function returns an error, causing the reconciliation
46+
// to requeue automatically via the controller-runtime framework.
47+
func updateGitStatusWithPatch(
48+
ctx context.Context,
49+
c client.Client,
50+
codebase *codebaseApi.Codebase,
51+
action codebaseApi.ActionType,
52+
gitStatus string,
53+
) error {
54+
// Skip update if status already matches (idempotency check)
55+
if codebase.Status.Git == gitStatus {
56+
return nil
57+
}
58+
59+
// Create patch based on current object state
60+
patch := client.MergeFrom(codebase.DeepCopy())
61+
62+
// Modify the status field
63+
codebase.Status.Git = gitStatus
64+
65+
// Apply patch to status subresource
66+
if err := c.Status().Patch(ctx, codebase, patch); err != nil {
67+
setFailedFields(codebase, action, err.Error())
68+
return fmt.Errorf("failed to patch git status to %s for codebase %s: %w",
69+
gitStatus, codebase.Name, err)
70+
}
71+
72+
return nil
73+
}

‎controllers/codebase/service/chain/common_test.go‎

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
package chain
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
"k8s.io/apimachinery/pkg/runtime"
11+
"k8s.io/apimachinery/pkg/types"
12+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
13+
14+
codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1"
15+
"github.com/epam/edp-codebase-operator/v2/pkg/util"
16+
)
17+
18+
func TestUpdateGitStatusWithPatch_Success(t *testing.T) {
19+
scheme := runtime.NewScheme()
20+
require.NoError(t, codebaseApi.AddToScheme(scheme))
21+
22+
codebase := &codebaseApi.Codebase{
23+
ObjectMeta: metav1.ObjectMeta{
24+
Name: fakeName,
25+
Namespace: fakeNamespace,
26+
},
27+
Status: codebaseApi.CodebaseStatus{
28+
Git: "",
29+
},
30+
}
31+
32+
fakeClient := fake.NewClientBuilder().
33+
WithScheme(scheme).
34+
WithObjects(codebase).
35+
WithStatusSubresource(codebase).
36+
Build()
37+
38+
err := updateGitStatusWithPatch(
39+
context.Background(),
40+
fakeClient,
41+
codebase,
42+
codebaseApi.RepositoryProvisioning,
43+
util.ProjectPushedStatus,
44+
)
45+
46+
assert.NoError(t, err)
47+
assert.Equal(t, util.ProjectPushedStatus, codebase.Status.Git)
48+
49+
// Verify status was actually patched in the cluster
50+
updated := &codebaseApi.Codebase{}
51+
err = fakeClient.Get(context.Background(), types.NamespacedName{
52+
Name: fakeName,
53+
Namespace: fakeNamespace,
54+
}, updated)
55+
assert.NoError(t, err)
56+
assert.Equal(t, util.ProjectPushedStatus, updated.Status.Git)
57+
}
58+
59+
func TestUpdateGitStatusWithPatch_Idempotent(t *testing.T) {
60+
scheme := runtime.NewScheme()
61+
require.NoError(t, codebaseApi.AddToScheme(scheme))
62+
63+
codebase := &codebaseApi.Codebase{
64+
ObjectMeta: metav1.ObjectMeta{
65+
Name: fakeName,
66+
Namespace: fakeNamespace,
67+
},
68+
Status: codebaseApi.CodebaseStatus{
69+
Git: util.ProjectPushedStatus,
70+
},
71+
}
72+
73+
fakeClient := fake.NewClientBuilder().
74+
WithScheme(scheme).
75+
WithObjects(codebase).
76+
WithStatusSubresource(codebase).
77+
Build()
78+
79+
err := updateGitStatusWithPatch(
80+
context.Background(),
81+
fakeClient,
82+
codebase,
83+
codebaseApi.RepositoryProvisioning,
84+
util.ProjectPushedStatus,
85+
)
86+
87+
assert.NoError(t, err)
88+
assert.Equal(t, util.ProjectPushedStatus, codebase.Status.Git)
89+
}
90+
91+
func TestUpdateGitStatusWithPatch_StatusTransition(t *testing.T) {
92+
// Setup
93+
scheme := runtime.NewScheme()
94+
require.NoError(t, codebaseApi.AddToScheme(scheme))
95+
96+
codebase := &codebaseApi.Codebase{
97+
ObjectMeta: metav1.ObjectMeta{
98+
Name: fakeName,
99+
Namespace: fakeNamespace,
100+
},
101+
Status: codebaseApi.CodebaseStatus{
102+
Git: util.ProjectPushedStatus,
103+
},
104+
}
105+
106+
fakeClient := fake.NewClientBuilder().
107+
WithScheme(scheme).
108+
WithObjects(codebase).
109+
WithStatusSubresource(codebase).
110+
Build()
111+
112+
// Execute - transition from ProjectPushedStatus to ProjectGitLabCIPushedStatus
113+
err := updateGitStatusWithPatch(
114+
context.Background(),
115+
fakeClient,
116+
codebase,
117+
codebaseApi.RepositoryProvisioning,
118+
util.ProjectGitLabCIPushedStatus,
119+
)
120+
121+
// Verify
122+
assert.NoError(t, err)
123+
assert.Equal(t, util.ProjectGitLabCIPushedStatus, codebase.Status.Git)
124+
125+
// Verify in cluster
126+
updated := &codebaseApi.Codebase{}
127+
err = fakeClient.Get(context.Background(), types.NamespacedName{
128+
Name: fakeName,
129+
Namespace: fakeNamespace,
130+
}, updated)
131+
assert.NoError(t, err)
132+
assert.Equal(t, util.ProjectGitLabCIPushedStatus, updated.Status.Git)
133+
}
134+
135+
func TestUpdateGitStatusWithPatch_PreservesOtherStatusFields(t *testing.T) {
136+
// Setup
137+
scheme := runtime.NewScheme()
138+
require.NoError(t, codebaseApi.AddToScheme(scheme))
139+
140+
codebase := &codebaseApi.Codebase{
141+
ObjectMeta: metav1.ObjectMeta{
142+
Name: fakeName,
143+
Namespace: fakeNamespace,
144+
},
145+
Status: codebaseApi.CodebaseStatus{
146+
Git: "",
147+
WebHookRef: "webhook-123",
148+
GitWebUrl: "https://git.example.com/repo",
149+
},
150+
}
151+
152+
fakeClient := fake.NewClientBuilder().
153+
WithScheme(scheme).
154+
WithObjects(codebase).
155+
WithStatusSubresource(codebase).
156+
Build()
157+
158+
// Execute
159+
err := updateGitStatusWithPatch(
160+
context.Background(),
161+
fakeClient,
162+
codebase,
163+
codebaseApi.RepositoryProvisioning,
164+
util.ProjectPushedStatus,
165+
)
166+
167+
// Verify - only Git field changed, others preserved
168+
assert.NoError(t, err)
169+
assert.Equal(t, util.ProjectPushedStatus, codebase.Status.Git)
170+
assert.Equal(t, "webhook-123", codebase.Status.WebHookRef)
171+
assert.Equal(t, "https://git.example.com/repo", codebase.Status.GitWebUrl)
172+
173+
// Verify in cluster
174+
updated := &codebaseApi.Codebase{}
175+
err = fakeClient.Get(context.Background(), types.NamespacedName{
176+
Name: fakeName,
177+
Namespace: fakeNamespace,
178+
}, updated)
179+
assert.NoError(t, err)
180+
assert.Equal(t, util.ProjectPushedStatus, updated.Status.Git)
181+
assert.Equal(t, "webhook-123", updated.Status.WebHookRef)
182+
assert.Equal(t, "https://git.example.com/repo", updated.Status.GitWebUrl)
183+
}
184+
185+
func TestUpdateGitStatusWithPatch_SequentialUpdates(t *testing.T) {
186+
// Setup - This test simulates the chain handler scenario
187+
scheme := runtime.NewScheme()
188+
require.NoError(t, codebaseApi.AddToScheme(scheme))
189+
190+
codebase := &codebaseApi.Codebase{
191+
ObjectMeta: metav1.ObjectMeta{
192+
Name: fakeName,
193+
Namespace: fakeNamespace,
194+
},
195+
Status: codebaseApi.CodebaseStatus{
196+
Git: "",
197+
},
198+
}
199+
200+
fakeClient := fake.NewClientBuilder().
201+
WithScheme(scheme).
202+
WithObjects(codebase).
203+
WithStatusSubresource(codebase).
204+
Build()
205+
206+
ctx := context.Background()
207+
208+
// Simulate sequential updates as in chain handlers
209+
// 1. PutProject sets ProjectPushedStatus
210+
err := updateGitStatusWithPatch(
211+
ctx,
212+
fakeClient,
213+
codebase,
214+
codebaseApi.RepositoryProvisioning,
215+
util.ProjectPushedStatus,
216+
)
217+
assert.NoError(t, err)
218+
assert.Equal(t, util.ProjectPushedStatus, codebase.Status.Git)
219+
220+
// 2. PutGitLabCIConfig sets ProjectGitLabCIPushedStatus
221+
err = updateGitStatusWithPatch(
222+
ctx,
223+
fakeClient,
224+
codebase,
225+
codebaseApi.RepositoryProvisioning,
226+
util.ProjectGitLabCIPushedStatus,
227+
)
228+
assert.NoError(t, err)
229+
assert.Equal(t, util.ProjectGitLabCIPushedStatus, codebase.Status.Git)
230+
231+
// 3. PutDeployConfigs sets ProjectTemplatesPushedStatus
232+
err = updateGitStatusWithPatch(
233+
ctx,
234+
fakeClient,
235+
codebase,
236+
codebaseApi.SetupDeploymentTemplates,
237+
util.ProjectTemplatesPushedStatus,
238+
)
239+
assert.NoError(t, err)
240+
assert.Equal(t, util.ProjectTemplatesPushedStatus, codebase.Status.Git)
241+
242+
// Verify final state in cluster
243+
updated := &codebaseApi.Codebase{}
244+
err = fakeClient.Get(ctx, types.NamespacedName{
245+
Name: fakeName,
246+
Namespace: fakeNamespace,
247+
}, updated)
248+
assert.NoError(t, err)
249+
assert.Equal(t, util.ProjectTemplatesPushedStatus, updated.Status.Git)
250+
}

‎controllers/codebase/service/chain/put_deploy_configs.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,8 @@ func (h *PutDeployConfigs) tryToPushConfigs(ctx context.Context, codebase *codeb
145145

146146
log.Info("Changes have been pushed")
147147

148-
codebase.Status.Git = util.ProjectTemplatesPushedStatus
149-
if err = h.client.Status().Update(ctx, codebase); err != nil {
150-
return fmt.Errorf("failed to set git status %s for codebase %s: %w", util.ProjectTemplatesPushedStatus, codebase.Name, err)
148+
if err = updateGitStatusWithPatch(ctx, h.client, codebase, codebaseApi.SetupDeploymentTemplates, util.ProjectTemplatesPushedStatus); err != nil {
149+
return err
151150
}
152151

153152
log.Info("Config has been pushed")

‎controllers/codebase/service/chain/put_gitlab_ci_config.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,8 @@ func (h *PutGitLabCIConfig) ServeRequest(ctx context.Context, codebase *codebase
5858
log.Info("End pushing GitLab CI config")
5959

6060
// Set status to mark this stage complete
61-
codebase.Status.Git = util.ProjectGitLabCIPushedStatus
62-
if err := h.client.Status().Update(ctx, codebase); err != nil {
63-
setFailedFields(codebase, codebaseApi.RepositoryProvisioning, err.Error())
64-
return fmt.Errorf("failed to set git status %s for codebase %s: %w",
65-
util.ProjectGitLabCIPushedStatus, codebase.Name, err)
61+
if err := updateGitStatusWithPatch(ctx, h.client, codebase, codebaseApi.RepositoryProvisioning, util.ProjectGitLabCIPushedStatus); err != nil {
62+
return err
6663
}
6764

6865
log.Info("GitLab CI status has been set successfully")

‎controllers/codebase/service/chain/put_project.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,8 @@ func (h *PutProject) ServeRequest(ctx context.Context, codebase *codebaseApi.Cod
9292
return fmt.Errorf("failed to create project: %w", err)
9393
}
9494

95-
codebase.Status.Git = util.ProjectPushedStatus
96-
if err = h.client.Status().Update(ctx, codebase); err != nil {
97-
setFailedFields(codebase, codebaseApi.RepositoryProvisioning, err.Error())
98-
return fmt.Errorf("failed to set git status %s for codebase %s: %w", util.ProjectPushedStatus, codebase.Name, err)
95+
if err = updateGitStatusWithPatch(ctx, h.client, codebase, codebaseApi.RepositoryProvisioning, util.ProjectPushedStatus); err != nil {
96+
return err
9997
}
10098

10199
log.Info("Finish putting project")

0 commit comments

Comments
 (0)