Skip to content

Commit 0a5e92e

Browse files
authored
reduce reconcilliation errors by not updating Status from GetPods() (#149)
1 parent 61c0ddd commit 0a5e92e

File tree

4 files changed

+5
-44
lines changed

4 files changed

+5
-44
lines changed

internal/controller/appwrapper/appwrapper_controller.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,18 +152,17 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
152152
}
153153
}
154154

155+
if err := awstatus.EnsureComponentStatusInitialized(ctx, aw); err != nil {
156+
return ctrl.Result{}, err
157+
}
158+
155159
return r.updateStatus(ctx, aw, workloadv1beta2.AppWrapperSuspended)
156160

157161
case workloadv1beta2.AppWrapperSuspended: // no components deployed
158162
if aw.Spec.Suspend {
159163
return ctrl.Result{}, nil // remain suspended
160164
}
161165

162-
// Normally already done as a side-effect of Kueue calling PodSets(), but be absolutely certain before we start using it.
163-
if err := awstatus.EnsureComponentStatusInitialized(ctx, aw); err != nil {
164-
return ctrl.Result{}, err
165-
}
166-
167166
// begin deployment
168167
meta.SetStatusCondition(&aw.Status.Conditions, metav1.Condition{
169168
Type: string(workloadv1beta2.QuotaReserved),

internal/controller/appwrapper/suite_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ import (
3939
"sigs.k8s.io/controller-runtime/pkg/log/zap"
4040

4141
workloadv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2"
42-
"github.com/project-codeflare/appwrapper/internal/controller/awstatus"
4342
kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
4443
)
4544

@@ -101,7 +100,6 @@ var _ = BeforeSuite(func() {
101100
k8sClient, err = client.New(cfg, client.Options{Scheme: scheme})
102101
Expect(err).NotTo(HaveOccurred())
103102
Expect(k8sClient).NotTo(BeNil())
104-
awstatus.CacheClient(k8sClient)
105103
})
106104

107105
var _ = AfterSuite(func() {

internal/controller/awstatus/status_utils.go

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,40 +21,9 @@ import (
2121

2222
workloadv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2"
2323
"github.com/project-codeflare/appwrapper/pkg/utils"
24-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2524
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
26-
"sigs.k8s.io/controller-runtime/pkg/client"
2725
)
2826

29-
var (
30-
cachedClient client.Client
31-
)
32-
33-
const controllerName = "workload.codeflare.dev-appwrapper"
34-
35-
// CacheClient initializes cachedClient; must be called during startup
36-
func CacheClient(k8sclient client.Client) {
37-
cachedClient = k8sclient
38-
}
39-
40-
// BaseSSAAppWrapper creates a new object based on the input AppWrapper that
41-
// only contains the fields necessary to identify the original object.
42-
// The object can be used as a base for Server-Side-Apply.
43-
func BaseSSAAppWrapper(aw *workloadv1beta2.AppWrapper) *workloadv1beta2.AppWrapper {
44-
patch := &workloadv1beta2.AppWrapper{
45-
ObjectMeta: metav1.ObjectMeta{
46-
UID: aw.UID,
47-
Name: aw.Name,
48-
Namespace: aw.Namespace,
49-
},
50-
TypeMeta: metav1.TypeMeta{
51-
APIVersion: workloadv1beta2.GroupVersion.String(),
52-
Kind: "AppWrapper",
53-
},
54-
}
55-
return patch
56-
}
57-
5827
// EnsureComponentStatusInitialized initializes aw.Status.ComponenetStatus, including performing PodSet inference for known GVKs
5928
func EnsureComponentStatusInitialized(ctx context.Context, aw *workloadv1beta2.AppWrapper) error {
6029
if len(aw.Status.ComponentStatus) == len(aw.Spec.Components) {
@@ -81,8 +50,5 @@ func EnsureComponentStatusInitialized(ctx context.Context, aw *workloadv1beta2.A
8150
}
8251
}
8352
aw.Status.ComponentStatus = compStatus
84-
85-
patch := BaseSSAAppWrapper(aw)
86-
patch.Status.ComponentStatus = compStatus
87-
return cachedClient.Status().Patch(ctx, patch, client.Apply, client.FieldOwner(controllerName), client.ForceOwnership)
53+
return nil
8854
}

pkg/controller/setup.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
cert "github.com/open-policy-agent/cert-controller/pkg/rotator"
3030

3131
"github.com/project-codeflare/appwrapper/internal/controller/appwrapper"
32-
"github.com/project-codeflare/appwrapper/internal/controller/awstatus"
3332
"github.com/project-codeflare/appwrapper/internal/controller/workload"
3433
"github.com/project-codeflare/appwrapper/internal/webhook"
3534
"github.com/project-codeflare/appwrapper/pkg/config"
@@ -39,7 +38,6 @@ import (
3938

4039
// SetupControllers creates and configures all components of the AppWrapper controller
4140
func SetupControllers(mgr ctrl.Manager, awConfig *config.AppWrapperConfig) error {
42-
awstatus.CacheClient(mgr.GetClient())
4341
if awConfig.EnableKueueIntegrations {
4442
if err := workload.WorkloadReconciler(
4543
mgr.GetClient(),

0 commit comments

Comments
 (0)