Skip to content

Commit 6b11294

Browse files
LeelaChachaLeelaChacha
authored
fix: SKRWebhook Condition Handling in Kyma CR (#2626)
* fix: SKRWebhook Condition Handling in Kyma CR * refactor: Make method a function * chore: Add test coverage * refactor: Test file * refactor * chore: coverage * refactor: nestif linter
1 parent 0bf61d6 commit 6b11294

File tree

4 files changed

+189
-5
lines changed

4 files changed

+189
-5
lines changed

‎internal/controller/kyma/controller.go‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -425,14 +425,14 @@ func (r *Reconciler) handleProcessingState(ctx context.Context, kyma *v1beta2.Ky
425425
errGroup.Go(func() error {
426426
if err := r.SKRWebhookManager.Reconcile(ctx, kyma); err != nil {
427427
r.Metrics.RecordRequeueReason(metrics.SkrWebhookResourcesInstallation, queue.UnexpectedRequeue)
428+
kyma.UpdateCondition(v1beta2.ConditionTypeSKRWebhook, apimetav1.ConditionFalse)
428429
if errors.Is(err, watcher.ErrSkrCertificateNotReady) {
429-
kyma.UpdateCondition(v1beta2.ConditionTypeSKRWebhook, apimetav1.ConditionFalse)
430430
return nil
431431
}
432432
return err
433433
}
434-
kyma.UpdateCondition(v1beta2.ConditionTypeSKRWebhook, apimetav1.ConditionTrue)
435-
return nil
434+
skrClient, _ := r.SkrContextFactory.Get(client.ObjectKeyFromObject(kyma))
435+
return checkSKRWebhookReadiness(ctx, skrClient, kyma)
436436
})
437437
}
438438

@@ -454,6 +454,19 @@ func (r *Reconciler) handleProcessingState(ctx context.Context, kyma *v1beta2.Ky
454454
r.updateStatus(ctx, kyma, state, "waiting for all modules to become ready")
455455
}
456456

457+
func checkSKRWebhookReadiness(ctx context.Context, skrClient *remote.SkrContext, kyma *v1beta2.Kyma) error {
458+
err := watcher.AssertDeploymentReady(ctx, skrClient)
459+
if err != nil {
460+
kyma.UpdateCondition(v1beta2.ConditionTypeSKRWebhook, apimetav1.ConditionFalse)
461+
if errors.Is(err, watcher.ErrSkrWebhookDeploymentInBackoff) {
462+
return err
463+
}
464+
return nil
465+
}
466+
kyma.UpdateCondition(v1beta2.ConditionTypeSKRWebhook, apimetav1.ConditionTrue)
467+
return nil
468+
}
469+
457470
func (r *Reconciler) handleDeletingState(ctx context.Context, kyma *v1beta2.Kyma) (ctrl.Result, error) {
458471
logger := logf.FromContext(ctx).V(log.InfoLevel)
459472

‎pkg/watcher/skr_webhook_manifest_manager.go‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77

88
"github.com/go-logr/logr"
9+
apiappsv1 "k8s.io/api/apps/v1"
910
apicorev1 "k8s.io/api/core/v1"
1011
apierrors "k8s.io/apimachinery/pkg/api/errors"
1112
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -24,10 +25,15 @@ import (
2425
skrwebhookresources "github.com/kyma-project/lifecycle-manager/pkg/watcher/skr_webhook_resources"
2526
)
2627

27-
var ErrSkrCertificateNotReady = errors.New("SKR certificate not ready")
28+
var (
29+
ErrSkrCertificateNotReady = errors.New("SKR certificate not ready")
30+
ErrSkrWebhookDeploymentNotReady = errors.New("SKR webhook deployment not ready")
31+
ErrSkrWebhookDeploymentInBackoff = errors.New("SKR webhook deployment in backoff state")
32+
)
2833

2934
const (
30-
skrChartFieldOwner = client.FieldOwner(shared.OperatorName)
35+
skrChartFieldOwner = client.FieldOwner(shared.OperatorName)
36+
skrWebhookDeploymentName = "skr-webhook"
3137
)
3238

3339
type WatcherMetrics interface {
@@ -294,3 +300,38 @@ func getWatchers(ctx context.Context, kcpClient client.Client) ([]v1beta2.Watche
294300

295301
return watcherList.Items, nil
296302
}
303+
304+
func AssertDeploymentReady(ctx context.Context, skrClient client.Reader) error {
305+
deployment := apiappsv1.Deployment{}
306+
deploymentKey := client.ObjectKey{
307+
Name: skrWebhookDeploymentName,
308+
Namespace: shared.DefaultRemoteNamespace,
309+
}
310+
if err := skrClient.Get(ctx, deploymentKey, &deployment); err != nil {
311+
return fmt.Errorf("failed to get skr-webhook deployment: %w", err)
312+
}
313+
314+
podList := &apicorev1.PodList{}
315+
err := skrClient.List(ctx, podList, client.InNamespace(shared.DefaultRemoteNamespace),
316+
client.MatchingLabels{"app": skrWebhookDeploymentName})
317+
if err != nil {
318+
return fmt.Errorf("failed to list pods: %w", err)
319+
}
320+
321+
if deploymentNotReady := deployment.Status.ReadyReplicas == 0; deploymentNotReady {
322+
// Check if pods are in backoff state
323+
for _, pod := range podList.Items {
324+
for _, cs := range pod.Status.ContainerStatuses {
325+
if cs.State.Waiting != nil && (cs.State.Waiting.Reason == "CrashLoopBackOff" ||
326+
cs.State.Waiting.Reason == "ImagePullBackOff") {
327+
return fmt.Errorf("%w: pod %s/%s in backoff state (%s)", ErrSkrWebhookDeploymentInBackoff,
328+
pod.Namespace, pod.Name, cs.State.Waiting.Reason)
329+
}
330+
}
331+
}
332+
333+
return fmt.Errorf("%w: deployment %s/%s is not in Ready state", ErrSkrWebhookDeploymentNotReady,
334+
deployment.Namespace, deployment.Name)
335+
}
336+
return nil
337+
}

‎pkg/watcher/skr_webhook_manifest_manager_test.go‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package watcher_test
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
apiappsv1 "k8s.io/api/apps/v1"
10+
apicorev1 "k8s.io/api/core/v1"
11+
"sigs.k8s.io/controller-runtime/pkg/client"
12+
13+
"github.com/kyma-project/lifecycle-manager/pkg/watcher"
14+
)
15+
16+
func TestAssertDeploymentReady_ReturnsNoError_WhenDeploymentReady(t *testing.T) {
17+
readyDeployment := &apiappsv1.Deployment{
18+
Status: apiappsv1.DeploymentStatus{
19+
ReadyReplicas: 1,
20+
},
21+
}
22+
getFunc := func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
23+
deployment, _ := obj.(*apiappsv1.Deployment)
24+
*deployment = *readyDeployment
25+
return nil
26+
}
27+
listFunc := func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
28+
return nil
29+
}
30+
mockClnt := &mockClient{getFunc: getFunc, listFunc: listFunc}
31+
ctx := t.Context()
32+
33+
err := watcher.AssertDeploymentReady(ctx, mockClnt)
34+
require.NoError(t, err)
35+
}
36+
37+
func TestAssertDeploymentReady_ReturnsError_WhenDeploymentNotReady(t *testing.T) {
38+
notReadyDeployment := &apiappsv1.Deployment{
39+
Status: apiappsv1.DeploymentStatus{
40+
ReadyReplicas: 0,
41+
},
42+
}
43+
getFunc := func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
44+
deployment, _ := obj.(*apiappsv1.Deployment)
45+
*deployment = *notReadyDeployment
46+
return nil
47+
}
48+
listFunc := func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
49+
return nil
50+
}
51+
mockClnt := &mockClient{getFunc: getFunc, listFunc: listFunc}
52+
ctx := t.Context()
53+
54+
err := watcher.AssertDeploymentReady(ctx, mockClnt)
55+
require.Error(t, err)
56+
require.ErrorIs(t, err, watcher.ErrSkrWebhookDeploymentNotReady)
57+
}
58+
59+
func TestAssertDeploymentReady_ReturnsError_WhenClientReturnsError(t *testing.T) {
60+
unexpectedError := errors.New("unexpected error")
61+
notFoundFunc := func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
62+
return unexpectedError
63+
}
64+
listFunc := func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
65+
return nil
66+
}
67+
mockClnt := &mockClient{getFunc: notFoundFunc, listFunc: listFunc}
68+
ctx := t.Context()
69+
70+
err := watcher.AssertDeploymentReady(ctx, mockClnt)
71+
require.Error(t, err)
72+
require.ErrorIs(t, err, unexpectedError)
73+
}
74+
75+
func TestAssertDeploymentReady_ReturnsError_WhenDeploymentInBackoff(t *testing.T) {
76+
deployment := &apiappsv1.Deployment{
77+
Status: apiappsv1.DeploymentStatus{
78+
ReadyReplicas: 0,
79+
},
80+
}
81+
podList := &apicorev1.PodList{
82+
Items: []apicorev1.Pod{{
83+
Status: apicorev1.PodStatus{
84+
ContainerStatuses: []apicorev1.ContainerStatus{{
85+
State: apicorev1.ContainerState{
86+
Waiting: &apicorev1.ContainerStateWaiting{
87+
Reason: "CrashLoopBackOff",
88+
},
89+
},
90+
}},
91+
},
92+
}},
93+
}
94+
getFunc := func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
95+
deploymentObj, ok := obj.(*apiappsv1.Deployment)
96+
if ok {
97+
*deploymentObj = *deployment
98+
}
99+
return nil
100+
}
101+
listFunc := func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
102+
podListObj, ok := list.(*apicorev1.PodList)
103+
if ok {
104+
*podListObj = *podList
105+
}
106+
return nil
107+
}
108+
mockClnt := &mockClient{getFunc: getFunc, listFunc: listFunc}
109+
ctx := t.Context()
110+
111+
err := watcher.AssertDeploymentReady(ctx, mockClnt)
112+
require.Error(t, err)
113+
require.ErrorIs(t, err, watcher.ErrSkrWebhookDeploymentInBackoff)
114+
}
115+
116+
// Stub for tests
117+
118+
type mockClient struct {
119+
getFunc func(context.Context, client.ObjectKey, client.Object, ...client.GetOption) error
120+
listFunc func(context.Context, client.ObjectList, ...client.ListOption) error
121+
}
122+
123+
func (m *mockClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
124+
return m.getFunc(ctx, key, obj, opts...)
125+
}
126+
127+
func (m *mockClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
128+
return m.listFunc(ctx, list, opts...)
129+
}

‎unit-test-coverage.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ packages:
3030
pkg/module/sync: 10
3131
pkg/templatelookup: 87
3232
pkg/templatelookup/moduletemplateinfolookup: 98
33+
pkg/watcher: 10
3334
pkg/watcher/skr_webhook_resources: 82
3435
pkg/watcher/certificate: 100
3536
pkg/watcher/certificate/certmanager: 100

0 commit comments

Comments
 (0)