Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change annotation cosmo-workspace.github.io/prune-disabled: true to cosmo-workspace.github.io/delete-policy: keep #825

Merged
merged 3 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions api/v1alpha1/instance_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ const (
LabelKeyInstanceName = "cosmo-workspace.github.io/instance"
// LabelKeyTemplateName is a template name label on the resources created by instance
LabelKeyTemplateName = "cosmo-workspace.github.io/template"

// ResourceAnnKeyDeletePolicy is a annotation key on the resources created by instance controller to specify delete policy
ResourceAnnKeyDeletePolicy = "cosmo-workspace.github.io/delete-policy"
// ResourceAnnEnumDeletePolicyDelete is delete policy, which controller do garbage collenction and attach owner references (default)
ResourceAnnEnumDeletePolicyDelete = "delete"
// ResourceAnnEnumDeletePolicyKeep is keep policy, which controller do not do garbage collenction and do not attach owner references
ResourceAnnEnumDeletePolicyKeep = "keep"
)

func init() {
Expand Down
17 changes: 4 additions & 13 deletions api/v1alpha1/utils.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package v1alpha1

import "strconv"

// LabelControllerManaged is a label on all resources managed by the controllers
const LabelControllerManaged = "cosmo-workspace.github.io/controller-managed"

Expand All @@ -26,22 +24,15 @@ type AnnotationHolder interface {
SetAnnotations(map[string]string)
}

// AnnotationPruneDisabled is a bool annotation for each child resources not to be deleted in GC
const AnnotationPruneDisabled = "cosmo-workspace.github.io/prune-disabled"

func IsPruneDisabled(obj AnnotationHolder) bool {
// KeepResourceDeletePolicy returns true if the resource has annotation delete-policy=keep
func KeepResourceDeletePolicy(obj AnnotationHolder) bool {
ann := obj.GetAnnotations()
if ann == nil {
return false
}
v, ok := ann[AnnotationPruneDisabled]
v, ok := ann[ResourceAnnKeyDeletePolicy]
if !ok {
return false
}
isDisabled, err := strconv.ParseBool(v)
if err != nil {
// invalid bool value might be accidentally set while trying to be true
return true
}
return isDisabled
return v == ResourceAnnEnumDeletePolicyKeep
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ spec:
apiVersion: v1
kind: Service
metadata:
annotations:
cosmo-workspace.github.io/delete-policy: keep
labels:
cosmo-workspace.github.io/instance: '{{INSTANCE}}'
cosmo-workspace.github.io/template: '{{TEMPLATE}}'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ spec:
apiVersion: v1
kind: Service
metadata:
annotations:
cosmo-workspace.github.io/delete-policy: keep
labels:
cosmo-workspace.github.io/instance: '{{INSTANCE}}'
cosmo-workspace.github.io/template: '{{TEMPLATE}}'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ apiVersion: v1
kind: Service
metadata:
name: workspace
annotations:
cosmo-workspace.github.io/delete-policy: keep
spec:
type: ClusterIP
ports:
Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/instance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (r *instanceReconciler) reconcileObjects(ctx context.Context, inst cosmov1a
}

// garbage collection
if len(errs) == 0 && !cosmov1alpha1.IsPruneDisabled(inst) {
if len(errs) == 0 && !cosmov1alpha1.KeepResourceDeletePolicy(inst) {
log.Debug().Info("checking garbage collection")
shouldDeletes := objectRefNotExistsInMap(lastApplied, currAppliedMap)
for _, d := range shouldDeletes {
Expand Down Expand Up @@ -297,7 +297,7 @@ func prune(ctx context.Context, r client.Client, d cosmov1alpha1.ObjectRef) (ski
log.Error(err, "target object UID is changed. skip pruning", "desiredUID", d.UID, "currentUID", obj.GetUID())
return true, nil
}
if cosmov1alpha1.IsPruneDisabled(&obj) {
if cosmov1alpha1.KeepResourceDeletePolicy(&obj) {
log.Debug().Info("skip pruning by annotation", "apiVersion")
return true, nil
}
Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/user_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
log.Info("status updated")
}

if user.Status.Phase != "AddonFailed" && !cosmov1alpha1.IsPruneDisabled(&user) {
if user.Status.Phase != "AddonFailed" && !cosmov1alpha1.KeepResourceDeletePolicy(&user) {
log.Debug().Info("checking garbage collection")
shouldDeletes := objectRefNotExistsInMap(lastAddons, currAddonsMap)
for _, d := range shouldDeletes {
Expand Down
23 changes: 22 additions & 1 deletion pkg/transformer/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@ package transformer

import (
"fmt"
"reflect"
"slices"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"

cosmov1alpha1 "github.com/cosmo-workspace/cosmo/api/v1alpha1"
"github.com/cosmo-workspace/cosmo/pkg/instance"
Expand Down Expand Up @@ -45,12 +50,28 @@ func (t *MetadataTransformer) Transform(src *unstructured.Unstructured) (*unstru
labels[cosmov1alpha1.LabelKeyTemplateName] = t.tmplName
obj.SetLabels(labels)

if !cosmov1alpha1.IsPruneDisabled(t.inst) && !cosmov1alpha1.IsPruneDisabled(obj) {
if !cosmov1alpha1.KeepResourceDeletePolicy(t.inst) && !cosmov1alpha1.KeepResourceDeletePolicy(obj) {
// Set owner reference
err := ctrl.SetControllerReference(t.inst, obj, t.scheme)
if err != nil {
return nil, fmt.Errorf("failed to set owner reference on %s: %w", obj.GetObjectKind().GroupVersionKind(), err)
}
} else {
// Remove owner reference
if len(obj.GetOwnerReferences()) > 0 {
gvk, _ := apiutil.GVKForObject(t.inst, t.scheme)
refs := slices.DeleteFunc(obj.GetOwnerReferences(), func(v metav1.OwnerReference) bool {
return reflect.DeepEqual(v, metav1.OwnerReference{
APIVersion: gvk.GroupVersion().String(),
Kind: gvk.Kind,
Name: t.inst.GetName(),
UID: t.inst.GetUID(),
Controller: ptr.To(true),
BlockOwnerDeletion: ptr.To(true),
})
})
obj.SetOwnerReferences(refs)
}
}
return obj, nil
}
138 changes: 136 additions & 2 deletions pkg/transformer/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,14 +271,14 @@ metadata:
wantErr: true,
},
{
name: "not put owner ref when prune-disabled annotated",
name: "not put owner ref when delete-policy: keep annotated",
fields: fields{
inst: &cosmov1alpha1.Instance{
ObjectMeta: metav1.ObjectMeta{
Name: "cs1",
Namespace: "cosmo-user-tom",
Annotations: map[string]string{
cosmov1alpha1.AnnotationPruneDisabled: "1",
cosmov1alpha1.ResourceAnnKeyDeletePolicy: cosmov1alpha1.ResourceAnnEnumDeletePolicyKeep,
},
},
Spec: cosmov1alpha1.InstanceSpec{
Expand Down Expand Up @@ -318,6 +318,140 @@ metadata:
namespace: cosmo-user-tom
spec:
host: example.com
`,
wantErr: false,
},
{
name: "Remove owner ref when delete-policy: keep annotated on Template resource",
fields: fields{
inst: &cosmov1alpha1.Instance{
ObjectMeta: metav1.ObjectMeta{
Name: "cs1",
Namespace: "cosmo-user-tom",
UID: "xxxxxxxxx",
},
Spec: cosmov1alpha1.InstanceSpec{
Template: cosmov1alpha1.TemplateRef{
Name: "code-server",
},
Override: cosmov1alpha1.OverrideSpec{},
Vars: map[string]string{"{{TEST}}": "OK"},
},
},
disableNamePrefix: false,
scheme: scheme,
},
args: args{
src: `apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
annotations:
cosmo/ingress-patch-enable: "true"
kubernetes.io/ingress.class: alb
cosmo-workspace.github.io/delete-policy: keep
labels:
cosmo-workspace.github.io/instance: cs1
cosmo-workspace.github.io/template: code-server
name: cs1-test
namespace: cosmo-user-tom
ownerReferences:
- apiVersion: cosmo-workspace.github.io/v1alpha1
blockOwnerDeletion: true
controller: true
kind: Instance
name: cs1
uid: "xxxxxxxxx"
- apiVersion: cosmo-workspace.github.io/v1alpha1
blockOwnerDeletion: true
controller: true
kind: Instance
name: cs1
uid: "yyyyyyyyy"
spec:
host: example.com
`,
},
want: `apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
annotations:
cosmo/ingress-patch-enable: "true"
kubernetes.io/ingress.class: alb
cosmo-workspace.github.io/delete-policy: keep
labels:
cosmo-workspace.github.io/instance: cs1
cosmo-workspace.github.io/template: code-server
name: cs1-test
namespace: cosmo-user-tom
ownerReferences:
- apiVersion: cosmo-workspace.github.io/v1alpha1
blockOwnerDeletion: true
controller: true
kind: Instance
name: cs1
uid: "yyyyyyyyy"
spec:
host: example.com
`,
wantErr: false,
},

{
name: "Remove owner ref when delete-policy: keep annotated on Instance",
fields: fields{
inst: &cosmov1alpha1.Instance{
ObjectMeta: metav1.ObjectMeta{
Name: "cs1",
Namespace: "cosmo-user-tom",
UID: "xxxxxxxxx",
Annotations: map[string]string{
cosmov1alpha1.ResourceAnnKeyDeletePolicy: cosmov1alpha1.ResourceAnnEnumDeletePolicyKeep,
},
},
Spec: cosmov1alpha1.InstanceSpec{
Template: cosmov1alpha1.TemplateRef{
Name: "code-server",
},
Override: cosmov1alpha1.OverrideSpec{},
Vars: map[string]string{"{{TEST}}": "OK"},
},
},
disableNamePrefix: false,
scheme: scheme,
},
args: args{
src: `apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
labels:
cosmo-workspace.github.io/instance: cs1
cosmo-workspace.github.io/template: code-server
name: cs1-test
namespace: cosmo-user-tom
ownerReferences:
- apiVersion: cosmo-workspace.github.io/v1alpha1
blockOwnerDeletion: true
controller: true
kind: Instance
name: cs1
uid: "xxxxxxxxx"
spec:
ingressClassName: alb
host: example.com
`,
},
want: `apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
labels:
cosmo-workspace.github.io/instance: cs1
cosmo-workspace.github.io/template: code-server
name: cs1-test
namespace: cosmo-user-tom
ownerReferences: []
spec:
ingressClassName: alb
host: example.com
`,
wantErr: false,
},
Expand Down
Loading