Skip to content
This repository has been archived by the owner on Jun 8, 2022. It is now read-only.

fix status automatically removed bug #212

Merged
merged 2 commits into from
Sep 21, 2020
Merged

fix status automatically removed bug #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
98cdad2
manually patch the extra fields in the appConfig status
ryanzhang-oss Sep 19, 2020
df59ba0
revert
ryanzhang-oss Sep 19, 2020
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
4 changes: 2 additions & 2 deletions apis/core/v1alpha2/core_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ type WorkloadStatus struct {
Status string `json:"status,omitempty"`

// HistoryWorkingRevision is a flag showing if it's history revision but still working
HistoryWorkingRevision bool `json:"currentWorkingRevision,omitempty"`
HistoryWorkingRevision bool `json:"historyWorkingRevision,omitempty"`

// ComponentName that produced this workload.
ComponentName string `json:"componentName,omitempty"`
Expand Down Expand Up @@ -401,7 +401,7 @@ type ApplicationConfigurationStatus struct {
// if it needs a single place to summarize the status of the entire application
Status ApplicationStatus `json:"status,omitempty"`

Dependency DependencyStatus `json:"dependency"`
Dependency DependencyStatus `json:"dependency,omitempty"`

// Workloads created by this ApplicationConfiguration.
Workloads []WorkloadStatus `json:"workloads,omitempty"`
Expand Down
4 changes: 1 addition & 3 deletions charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ spec:
componentRevisionName:
description: ComponentRevisionName of current component
type: string
currentWorkingRevision:
historyWorkingRevision:
description: HistoryWorkingRevision is a flag showing if it's
history revision but still working
type: boolean
Expand Down Expand Up @@ -486,8 +486,6 @@ spec:
type: object
type: object
type: array
required:
- dependency
type: object
type: object
served: true
Expand Down
41 changes: 33 additions & 8 deletions pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco
if err := r.client.Get(ctx, req.NamespacedName, ac); err != nil {
return reconcile.Result{}, errors.Wrap(resource.IgnoreNotFound(err), errGetAppConfig)
}
acPatch := ac.DeepCopy()

if ac.ObjectMeta.DeletionTimestamp.IsZero() {
if registerFinalizers(ac) {
Expand Down Expand Up @@ -296,10 +297,8 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco
record.Event(ac, event.Normal(reasonGGComponent, "Successfully garbage collected component"))
}

// patch the final status
acPatch := client.MergeFrom(ac.DeepCopyObject())

r.updateStatus(ctx, ac, workloads)
// patch the final status on the client side, k8s sever can't merge them
r.updateStatus(ctx, ac, acPatch, workloads)

ac.Status.Dependency = v1alpha2.DependencyStatus{}
waitTime := longWait
Expand All @@ -308,11 +307,11 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco
ac.Status.Dependency = *depStatus
}

return reconcile.Result{RequeueAfter: waitTime},
errors.Wrap(r.client.Status().Patch(ctx, ac, acPatch, client.FieldOwner(ac.GetUID())), errUpdateAppConfigStatus)
// the posthook function will do the final status update
return reconcile.Result{RequeueAfter: waitTime}, nil
}

func (r *OAMApplicationReconciler) updateStatus(ctx context.Context, ac *v1alpha2.ApplicationConfiguration, workloads []Workload) {
func (r *OAMApplicationReconciler) updateStatus(ctx context.Context, ac, acPatch *v1alpha2.ApplicationConfiguration, workloads []Workload) {
ac.Status.Workloads = make([]v1alpha2.WorkloadStatus, len(workloads))
revisionStatus := make([]v1alpha2.WorkloadStatus, 0)
for i, w := range workloads {
Expand Down Expand Up @@ -346,10 +345,36 @@ func (r *OAMApplicationReconciler) updateStatus(ctx context.Context, ac *v1alpha
}
}
ac.Status.Workloads = append(ac.Status.Workloads, revisionStatus...)

// patch the extra fields in the status that is wiped by the Status() function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status() is not specific enough to pinpoint the code location.
Better point out its parent function/object as well.

patchExtraStatusField(&ac.Status, acPatch.Status)
ac.SetConditions(v1alpha1.ReconcileSuccess())
}

func patchExtraStatusField(acStatus *v1alpha2.ApplicationConfigurationStatus, acPatchStatus v1alpha2.ApplicationConfigurationStatus) {
wonderflow marked this conversation as resolved.
Show resolved Hide resolved
// patch the extra status back
for i := range acStatus.Workloads {
for _, w := range acPatchStatus.Workloads {
// find the workload in the old status
if acStatus.Workloads[i].ComponentRevisionName == w.ComponentRevisionName {
if len(w.Status) > 0 {
acStatus.Workloads[i].Status = w.Status
}
//find the trait
for j := range acStatus.Workloads[i].Traits {
for _, t := range w.Traits {
tr := acStatus.Workloads[i].Traits[j].Reference
if t.Reference.APIVersion == tr.APIVersion && t.Reference.Kind == tr.Kind && t.Reference.Name == tr.Name {
if len(t.Status) > 0 {
acStatus.Workloads[i].Traits[j].Status = t.Status
}
}
}
}
}
}
}
}

// if any finalizers newly registered, return true
func registerFinalizers(ac *v1alpha2.ApplicationConfiguration) bool {
newFinalizer := false
Expand Down
189 changes: 189 additions & 0 deletions pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1394,3 +1394,192 @@ func TestAddDataOutputsToDAG(t *testing.T) {
t.Errorf("didn't add conditions to source correctly: %s", diff)
}
}

func TestPatchExtraField(t *testing.T) {
tests := map[string]struct {
acStatus *v1alpha2.ApplicationConfigurationStatus
acPatchStatus v1alpha2.ApplicationConfigurationStatus
wantedStatus *v1alpha2.ApplicationConfigurationStatus
}{
"patch extra": {
acStatus: &v1alpha2.ApplicationConfigurationStatus{
Workloads: []v1alpha2.WorkloadStatus{
{
HistoryWorkingRevision: false,
ComponentName: "test",
ComponentRevisionName: "test-v1",
Traits: []v1alpha2.WorkloadTrait{
{
Reference: runtimev1alpha1.TypedReference{
APIVersion: "apiVersion1",
Kind: "kind1",
Name: "trait1",
},
},
},
},
},
},
acPatchStatus: v1alpha2.ApplicationConfigurationStatus{
Workloads: []v1alpha2.WorkloadStatus{
{
Status: "we need to add this",
ComponentRevisionName: "test-v1",
Traits: []v1alpha2.WorkloadTrait{
{
Status: "add this too",
Reference: runtimev1alpha1.TypedReference{
APIVersion: "apiVersion1",
Kind: "kind1",
Name: "trait1",
},
},
},
},
},
},
wantedStatus: &v1alpha2.ApplicationConfigurationStatus{
Workloads: []v1alpha2.WorkloadStatus{
{
Status: "we need to add this",
HistoryWorkingRevision: false,
ComponentName: "test",
ComponentRevisionName: "test-v1",
Traits: []v1alpha2.WorkloadTrait{
{
Status: "add this too",
Reference: runtimev1alpha1.TypedReference{
APIVersion: "apiVersion1",
Kind: "kind1",
Name: "trait1",
},
},
},
},
},
},
},
"patch trait mismatch": {
acStatus: &v1alpha2.ApplicationConfigurationStatus{
Workloads: []v1alpha2.WorkloadStatus{
{
HistoryWorkingRevision: false,
ComponentName: "test",
ComponentRevisionName: "test-v1",
Traits: []v1alpha2.WorkloadTrait{
{
Reference: runtimev1alpha1.TypedReference{
APIVersion: "apiVersion1",
Kind: "kind1",
Name: "trait1",
},
},
},
},
},
},
acPatchStatus: v1alpha2.ApplicationConfigurationStatus{
Workloads: []v1alpha2.WorkloadStatus{
{
Status: "we need to add this",
ComponentRevisionName: "test-v1",
Traits: []v1alpha2.WorkloadTrait{
{
Status: "add this too",
Reference: runtimev1alpha1.TypedReference{
APIVersion: "apiVersion1",
Kind: "kind1",
Name: "trait2",
},
},
},
},
},
},
wantedStatus: &v1alpha2.ApplicationConfigurationStatus{
Workloads: []v1alpha2.WorkloadStatus{
{
Status: "we need to add this",
HistoryWorkingRevision: false,
ComponentName: "test",
ComponentRevisionName: "test-v1",
Traits: []v1alpha2.WorkloadTrait{
{
Reference: runtimev1alpha1.TypedReference{
APIVersion: "apiVersion1",
Kind: "kind1",
Name: "trait1",
},
},
},
},
},
},
},
"patch workload revision mismatch": {
acStatus: &v1alpha2.ApplicationConfigurationStatus{
Workloads: []v1alpha2.WorkloadStatus{
{
HistoryWorkingRevision: false,
ComponentName: "test",
ComponentRevisionName: "test-v1",
Traits: []v1alpha2.WorkloadTrait{
{
Reference: runtimev1alpha1.TypedReference{
APIVersion: "apiVersion1",
Kind: "kind1",
Name: "trait1",
},
},
},
},
},
},
acPatchStatus: v1alpha2.ApplicationConfigurationStatus{
Workloads: []v1alpha2.WorkloadStatus{
{
Status: "we need to add this",
ComponentRevisionName: "test-v2",
Traits: []v1alpha2.WorkloadTrait{
{
Status: "add this too",
Reference: runtimev1alpha1.TypedReference{
APIVersion: "apiVersion1",
Kind: "kind1",
Name: "trait1",
},
},
},
},
},
},
wantedStatus: &v1alpha2.ApplicationConfigurationStatus{
Workloads: []v1alpha2.WorkloadStatus{
{
HistoryWorkingRevision: false,
ComponentName: "test",
ComponentRevisionName: "test-v1",
Traits: []v1alpha2.WorkloadTrait{
{
Reference: runtimev1alpha1.TypedReference{
APIVersion: "apiVersion1",
Kind: "kind1",
Name: "trait1",
},
},
},
},
},
},
},
}
for testName, tt := range tests {
t.Run(testName, func(t *testing.T) {
patchExtraStatusField(tt.acStatus, tt.acPatchStatus)
if diff := cmp.Diff(tt.acStatus, tt.wantedStatus); diff != "" {
t.Errorf("didn't patch to the statsu correctly: %s", diff)
}

})
}
}
Loading