Skip to content

Commit 3d4daa5

Browse files
author
Amit Kumar Das
authored
feat(recipe): garbage collect the lock when its recipe is deleted (#120)
In addition this commit has following changes: - several api changes to kind: Recipe - test/e2e/ci.yaml enhancements - test/e2e/inference.yaml enhancements - test/e2e/suite.sh enhancements - custom columns for kind: Recipe Signed-off-by: AmitKumarDas <[email protected]>
1 parent ebd8afa commit 3d4daa5

27 files changed

+834
-257
lines changed

cmd/main.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,11 @@ func main() {
6262

6363
// controller name & corresponding controller reconcile function
6464
var controllers = map[string]generic.InlineInvokeFn{
65-
"sync/recipe": recipe.Sync,
66-
"sync/http": http.Sync,
67-
"sync/doperator": doperator.Sync,
68-
"sync/run": run.Sync,
65+
"sync/recipe": recipe.Sync,
66+
"finalize/recipe": recipe.Finalize,
67+
"sync/http": http.Sync,
68+
"sync/doperator": doperator.Sync,
69+
"sync/run": run.Sync,
6970
}
7071
for name, ctrl := range controllers {
7172
generic.AddToInlineRegistry(name, ctrl)

common/controller/reconciler.go

+17-16
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"fmt"
2121
"strings"
2222

23-
"github.com/golang/glog"
23+
"k8s.io/klog/v2"
2424
"openebs.io/metac/controller/generic"
2525

2626
metacutil "mayadata.io/d-operators/common/metac"
@@ -57,17 +57,17 @@ func (r *Reconciler) validateHook() {
5757

5858
// logSyncStart logs the start of sync
5959
func (r *Reconciler) logSyncStart() {
60-
glog.V(4).Infof(
61-
"Starting sync of %s: %s",
60+
klog.V(3).Infof(
61+
"Starting sync: Controller %q: %s",
6262
r.Name,
6363
metacutil.GetDetailsFromRequest(r.HookRequest),
6464
)
6565
}
6666

6767
// logSyncFinish logs the completion of sync
6868
func (r *Reconciler) logSyncFinish() {
69-
glog.V(4).Infof(
70-
"Completed sync of %s: %s: %s",
69+
klog.V(3).Infof(
70+
"Completed sync: Controller %q: Request %s: Response %s",
7171
r.Name,
7272
metacutil.GetDetailsFromRequest(r.HookRequest),
7373
metacutil.GetDetailsFromResponse(r.HookResponse),
@@ -115,10 +115,9 @@ func (r *Reconciler) handleError() {
115115
return
116116
}
117117
// log this error with context
118-
glog.Errorf(
119-
"Failed to sync %s: Watch of kind=%s name=%s/%s: %s",
118+
klog.Errorf(
119+
"Reconcile failed: Controller %q: Name %q %q: Error %s",
120120
r.Name,
121-
r.HookRequest.Watch.GetKind(),
122121
r.HookRequest.Watch.GetNamespace(),
123122
r.HookRequest.Watch.GetName(),
124123
r.Err.Error(),
@@ -146,11 +145,11 @@ func (r *Reconciler) Reconcile() error {
146145
r.updateWatchStatus,
147146
}
148147
}
149-
var allFns = []func(){}
150-
allFns = append(allFns, r.PreReconcileFns...)
151-
allFns = append(allFns, r.ReconcileFns...)
152-
allFns = append(allFns, r.PostReconcileFns...)
153-
for _, fn := range allFns {
148+
var reconFns = []func(){}
149+
reconFns = append(reconFns, r.PreReconcileFns...)
150+
reconFns = append(reconFns, r.ReconcileFns...)
151+
reconFns = append(reconFns, r.PostReconcileFns...)
152+
for _, fn := range reconFns {
154153
fn()
155154
// post operation checks
156155
if r.Fatal != nil {
@@ -161,18 +160,20 @@ func (r *Reconciler) Reconcile() error {
161160
// this logs the error thus avoiding panic in the
162161
// controller
163162
r.handleError()
163+
// break out of the reconcile functions
164164
break
165165
}
166166
}
167+
// desired watch functions are run even if reconcile functions
168+
// result in any error
167169
for _, dWatchFn := range r.DesiredWatchFns {
168170
dWatchFn()
169171
}
170172
// check if attachments / children need not be reconciled
171173
if r.HookResponse.SkipReconcile {
172-
glog.V(3).Infof(
173-
"Skipping sync of %s: Watch of kind=%s name=%s/%s: Reason=%s",
174+
klog.V(3).Infof(
175+
"Will skip reconcile: Controller %q: Name %q %q: %s",
174176
r.Name,
175-
r.HookRequest.Watch.GetKind(),
176177
r.HookRequest.Watch.GetNamespace(),
177178
r.HookRequest.Watch.GetName(),
178179
r.SkipReason,

common/unstruct/unstruct.go

+31
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/pkg/errors"
2121
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2222
"k8s.io/apimachinery/pkg/runtime"
23+
"k8s.io/apimachinery/pkg/util/json"
2324
)
2425

2526
// ToTyped transforms the provided unstruct instance
@@ -40,3 +41,33 @@ func ToTyped(src *unstructured.Unstructured, target interface{}) error {
4041
target,
4142
)
4243
}
44+
45+
// MarshalThenUnmarshal marshals the provided src and unmarshals
46+
// it back into the dest
47+
func MarshalThenUnmarshal(src map[string]interface{}, dest interface{}) error {
48+
data, err := json.Marshal(src)
49+
if err != nil {
50+
return err
51+
}
52+
return json.Unmarshal(data, dest)
53+
}
54+
55+
// SetLabels updates the given labels with the ones
56+
// found in the provided unstructured instance
57+
func SetLabels(obj *unstructured.Unstructured, lbls map[string]string) {
58+
if len(lbls) == 0 {
59+
return
60+
}
61+
if obj == nil || obj.Object == nil {
62+
return
63+
}
64+
got := obj.GetLabels()
65+
if got == nil {
66+
got = make(map[string]string)
67+
}
68+
for k, v := range lbls {
69+
// update given label against existing
70+
got[k] = v
71+
}
72+
obj.SetLabels(got)
73+
}

config/metac.yaml

+26-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ metadata:
44
name: sync-recipe
55
namespace: dope
66
spec:
7-
watch:
8-
# kind: Recipe custom resource is watched
7+
watch:
98
apiVersion: dope.mayadata.io/v1
109
resource: recipes
1110
hooks:
@@ -15,6 +14,31 @@ spec:
1514
---
1615
apiVersion: dope/v1
1716
kind: GenericController
17+
metadata:
18+
name: finalize-recipe
19+
namespace: dope
20+
spec:
21+
watch:
22+
apiVersion: dope.mayadata.io/v1
23+
resource: recipes
24+
attachments:
25+
- apiVersion: v1
26+
resource: configmaps
27+
advancedSelector:
28+
selectorTerms:
29+
# select ConfigMap if its labels has following
30+
- matchLabels:
31+
recipe.dope.mayadata.io/lock: "true"
32+
matchReferenceExpressions:
33+
- key: metadata.labels.recipe\.dope\.mayadata\.io/name
34+
operator: EqualsWatchName # match this lbl value against watch Name
35+
hooks:
36+
finalize:
37+
inline:
38+
funcName: finalize/recipe
39+
---
40+
apiVersion: dope/v1
41+
kind: GenericController
1842
metadata:
1943
name: sync-http
2044
namespace: dope

controller/recipe/finalizer.go

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
Copyright 2020 The MayaData Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
https://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package recipe
18+
19+
import (
20+
"openebs.io/metac/controller/generic"
21+
)
22+
23+
// Finalize implements the idempotent logic that gets executed when
24+
// Recipe instance is deleted. A Recipe instance is associated with
25+
// a dedicated lock in form of a ConfigMap. Finalize logic tries to
26+
// delete this ConfigMap.
27+
//
28+
// NOTE:
29+
// When finalize hook is set in the config metac automatically sets
30+
// a finalizer entry against the Recipe metadata's finalizers field .
31+
// This finalizer entry is removed when SyncHookResponse's Finalized
32+
// field is set to true.
33+
//
34+
// NOTE:
35+
// SyncHookRequest is the payload received as part of finalize
36+
// request. Similarly, SyncHookResponse is the payload sent as a
37+
// response as part of finalize request.
38+
//
39+
// NOTE:
40+
// Returning error will panic this process. We would rather want this
41+
// controller to run continuously. Hence, the errors are handled.
42+
func Finalize(request *generic.SyncHookRequest, response *generic.SyncHookResponse) error {
43+
if request.Attachments.IsEmpty() {
44+
// Since no ConfigMap is found it is safe to delete Recipe
45+
response.Finalized = true
46+
return nil
47+
}
48+
// A single ConfigMap instance is expected in the attachments
49+
// That needs to be deleted explicitly
50+
response.ExplicitDeletes = request.Attachments.List()
51+
return nil
52+
}

controller/recipe/reconciler.go

+13-47
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package recipe
1818

1919
import (
20-
"k8s.io/utils/pointer"
2120
"openebs.io/metac/controller/generic"
2221
k8s "openebs.io/metac/third_party/kubernetes"
2322

@@ -52,73 +51,40 @@ func (r *Reconciler) invoke() {
5251
Recipe: *r.ObservedRecipe,
5352
},
5453
)
55-
r.RecipeStatus, r.Err = runner.Run()
54+
r.Err = runner.Run()
5655
}
5756

5857
func (r *Reconciler) setSyncResponse() {
5958
// we skip the reconcile always since there are no attachments
6059
// to reconcile
6160
r.HookResponse.SkipReconcile = true
61+
// default skip reason
6262
r.SkipReason = "No attachments to reconcile"
63-
// update the skip reason for locked recipes
64-
if r.RecipeStatus.Phase == types.RecipeStatusLocked {
65-
r.SkipReason = r.RecipeStatus.Reason
66-
}
67-
// set resync period for recipes with errors
68-
if r.Err != nil {
69-
// resync since this might be a temporary error
70-
//
71-
// TODO:
72-
// Might be better to expose this from recipe.spec
73-
r.HookResponse.ResyncAfterSeconds = 5.0
74-
}
7563
}
7664

7765
func (r *Reconciler) setRecipeStatusAsError() {
66+
if r.ObservedRecipe != nil &&
67+
r.ObservedRecipe.Spec.Refresh.OnErrorResyncAfterSeconds != nil {
68+
// resync on error based on configuration
69+
r.HookResponse.ResyncAfterSeconds =
70+
*r.ObservedRecipe.Spec.Refresh.OnErrorResyncAfterSeconds
71+
}
7872
r.HookResponse.Status = map[string]interface{}{
7973
"phase": "Error",
8074
"reason": r.Err.Error(),
8175
}
8276
r.HookResponse.Labels = map[string]*string{
83-
"recipe.dope.mayadata.io/phase": k8s.StringPtr("Error"),
77+
types.LblKeyRecipePhase: k8s.StringPtr("Error"),
8478
}
8579
}
8680

8781
func (r *Reconciler) setRecipeStatus() {
88-
r.HookResponse.Status = map[string]interface{}{
89-
"phase": string(r.RecipeStatus.Phase),
90-
"reason": r.RecipeStatus.Reason,
91-
"message": r.RecipeStatus.Message,
92-
"failedTaskCount": int64(r.RecipeStatus.FailedTaskCount),
93-
"taskCount": int64(r.RecipeStatus.TaskCount),
94-
"taskListStatus": r.RecipeStatus.TaskListStatus,
95-
}
96-
r.HookResponse.Labels = map[string]*string{
97-
"recipe.dope.mayadata.io/phase": pointer.StringPtr(string(r.RecipeStatus.Phase)),
98-
}
99-
if r.ObservedRecipe != nil &&
100-
r.ObservedRecipe.Spec.Refresh.ResyncAfterSeconds != nil {
101-
r.HookResponse.ResyncAfterSeconds = *r.ObservedRecipe.Spec.Refresh.ResyncAfterSeconds
102-
}
103-
}
104-
105-
func (r *Reconciler) setWatchStatus() {
10682
if r.Err != nil {
107-
if r.ObservedRecipe != nil &&
108-
r.ObservedRecipe.Spec.Refresh.OnErrorResyncAfterSeconds != nil {
109-
// resync based on configuration
110-
r.HookResponse.ResyncAfterSeconds =
111-
*r.ObservedRecipe.Spec.Refresh.OnErrorResyncAfterSeconds
112-
}
83+
// reconciler is only concerned about the Recipes that
84+
// result in error
11385
r.setRecipeStatusAsError()
11486
return
11587
}
116-
if r.RecipeStatus.Phase == types.RecipeStatusLocked {
117-
// nothing needs to be done
118-
// old status will persist
119-
return
120-
}
121-
r.setRecipeStatus()
12288
}
12389

12490
// Sync implements the idempotent logic to sync Recipe resource
@@ -133,7 +99,7 @@ func (r *Reconciler) setWatchStatus() {
13399
func Sync(request *generic.SyncHookRequest, response *generic.SyncHookResponse) error {
134100
r := &Reconciler{
135101
Reconciler: commonctrl.Reconciler{
136-
Name: "recipe-sync-reconciler",
102+
Name: "sync-recipe",
137103
HookRequest: request,
138104
HookResponse: response,
139105
},
@@ -146,7 +112,7 @@ func Sync(request *generic.SyncHookRequest, response *generic.SyncHookResponse)
146112
}
147113
// add functions to achieve desired watch
148114
r.DesiredWatchFns = []func(){
149-
r.setWatchStatus,
115+
r.setRecipeStatus,
150116
}
151117
// run reconcile
152118
return r.Reconcile()

go.mod

+2-3
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,19 @@ go 1.13
55
require (
66
github.com/go-cmd/cmd v1.2.0
77
github.com/go-resty/resty/v2 v2.2.0
8-
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
98
github.com/google/go-cmp v0.4.0
109
github.com/pkg/errors v0.9.1
1110
k8s.io/apiextensions-apiserver v0.17.3
1211
k8s.io/apimachinery v0.17.3
1312
k8s.io/client-go v0.17.3
1413
k8s.io/klog/v2 v2.0.0
1514
k8s.io/utils v0.0.0-20191114184206-e782cd3c129f
16-
openebs.io/metac v0.3.0
15+
openebs.io/metac v0.4.0
1716
)
1817

1918
replace (
2019
k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.17.3
2120
k8s.io/apimachinery => k8s.io/apimachinery v0.17.3
2221
k8s.io/client-go => k8s.io/client-go v0.17.3
23-
openebs.io/metac => github.com/AmitKumarDas/metac v0.3.0
22+
openebs.io/metac => github.com/AmitKumarDas/metac v0.4.0
2423
)

go.sum

+2-3
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMT
33
cloud.google.com/go v0.38.0/go.mod h1:990N+gfupTy94rShfmMCWGDn0LpTmnzTp2qbd1dvSRU=
44
contrib.go.opencensus.io/exporter/prometheus v0.1.0 h1:SByaIoWwNgMdPSgl5sMqM2KDE5H/ukPWBRo314xiDvg=
55
contrib.go.opencensus.io/exporter/prometheus v0.1.0/go.mod h1:cGFniUXGZlKRjzOyuZJ6mgB+PgBcCIa79kEKR8YCW+A=
6-
github.com/AmitKumarDas/metac v0.3.0 h1:4pKqhty7suyd/xdYv0i2UIeXMNlR8Ofs42Ac2EGr9B4=
7-
github.com/AmitKumarDas/metac v0.3.0/go.mod h1:a4xvzoLnA372zieEE0d/P7tJtZE5jqLcpn0mJnXqcSs=
6+
github.com/AmitKumarDas/metac v0.4.0 h1:DicLgezoHVoYrC0wtatCecfZoR93Vb++yTsT6zXQAQM=
7+
github.com/AmitKumarDas/metac v0.4.0/go.mod h1:OdHjrNt6jJ6e723kuQB5Bou2noiU4l3FfzEgzVNWJtk=
88
github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
99
github.com/Azure/go-autorest/autorest v0.9.0/go.mod h1:xyHB1BMZT0cuDHU7I0+g046+BFDTQ8rEZB0s4Yfa6bI=
1010
github.com/Azure/go-autorest/autorest/adal v0.5.0/go.mod h1:8Z9fGy2MpX0PvDjB1pEgQTmVqjGhiHBW7RJJEciWzS0=
@@ -37,7 +37,6 @@ github.com/blang/semver v3.5.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnweb
3737
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
3838
github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8=
3939
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
40-
github.com/coreos/etcd v3.3.15+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
4140
github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=
4241
github.com/coreos/go-oidc v2.1.0+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc=
4342
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=

0 commit comments

Comments
 (0)