Skip to content

Commit

Permalink
Merge pull request #770 from jlandowner/feature/useraddon-gc
Browse files Browse the repository at this point in the history 
GC for User addon
  • Loading branch information
@oruharo
oruharo authored Jun 21, 2023
2 parents 1688854 + c3fad88 commit 748d334
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 124 deletions.
39 changes: 6 additions & 33 deletions internal/controllers/__snapshots__/user_controller_test.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ SnapShot = """
\"addons\": [
{
\"kind\": \"Instance\",
\"namespace\": \"cosmo-user-ua\",
\"name\": \"useraddon-namespaced-addon\",
\"apiVersion\": \"cosmo-workspace.github.io/v1alpha1\"
},
Expand All @@ -48,7 +49,7 @@ SnapShot = """
}
"""
['User controller when updating user addon with invalid addon should try to create addon but status AddonFailed 1']
['User controller when updating user addon should gc old addon and try to create new addon 1']
SnapShot = """
{
\"metadata\": {
Expand All @@ -64,13 +65,7 @@ SnapShot = """
\"name\": \"namespaced-addon\"
},
\"vars\": {
\"IMAGE_TAG\": \"v0.71.0\"
}
},
{
\"template\": {
\"name\": \"cluster-addon\",
\"clusterScoped\": true
\"KEY\": \"VAL\"
}
},
{
Expand All @@ -90,6 +85,7 @@ SnapShot = """
\"addons\": [
{
\"kind\": \"Instance\",
\"namespace\": \"cosmo-user-ua\",
\"name\": \"useraddon-namespaced-addon\",
\"apiVersion\": \"cosmo-workspace.github.io/v1alpha1\"
},
Expand All @@ -103,7 +99,7 @@ SnapShot = """
}
"""
['User controller when updating user addon with invalid addon should try to create addon but status AddonFailed 2']
['User controller when updating user addon should gc old namespaced addon 1']
SnapShot = """
{
\"metadata\": {
Expand All @@ -114,20 +110,6 @@ SnapShot = """
\"displayName\": \"お名前\",
\"authType\": \"password-secret\",
\"addons\": [
{
\"template\": {
\"name\": \"namespaced-addon\"
},
\"vars\": {
\"IMAGE_TAG\": \"v0.71.0\"
}
},
{
\"template\": {
\"name\": \"cluster-addon\",
\"clusterScoped\": true
}
},
{
\"template\": {
\"name\": \"empty-addon\"
Expand All @@ -145,18 +127,9 @@ SnapShot = """
\"addons\": [
{
\"kind\": \"Instance\",
\"namespace\": \"cosmo-user-ua\",
\"name\": \"useraddon-empty-addon\",
\"apiVersion\": \"cosmo-workspace.github.io/v1alpha1\"
},
{
\"kind\": \"Instance\",
\"name\": \"useraddon-namespaced-addon\",
\"apiVersion\": \"cosmo-workspace.github.io/v1alpha1\"
},
{
\"kind\": \"ClusterInstance\",
\"name\": \"useraddon-ua-cluster-addon\",
\"apiVersion\": \"cosmo-workspace.github.io/v1alpha1\"
}
]
}
Expand Down
9 changes: 6 additions & 3 deletions internal/controllers/instance_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,16 @@ spec:
By("fetching instance resource and checking if last applied resources added in instance status")

var createdInst cosmov1alpha1.Instance
Eventually(func() error {
Eventually(func() int {
key := client.ObjectKey{
Name: inst.Name,
Namespace: inst.Namespace,
}
return k8sClient.Get(ctx, key, &createdInst)
}, time.Second*10).Should(Succeed())
err = k8sClient.Get(ctx, key, &createdInst)
Expect(err).ShouldNot(HaveOccurred())

return createdInst.Status.LastAppliedObjectsCount
}, time.Second*10).ShouldNot(BeZero())
Ω(InstanceSnapshot(&createdInst)).To(MatchSnapShot())
})
})
Expand Down
120 changes: 71 additions & 49 deletions internal/controllers/user_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
apierrors "k8s.io/apimachinery/pkg/api/errors"
apierrs "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -74,15 +77,6 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
CreationTimestamp: &ns.CreationTimestamp,
}

// update user status
if !equality.Semantic.DeepEqual(currentUser, &user) {
log.Debug().PrintObjectDiff(currentUser, &user)
if err := r.Status().Update(ctx, &user); err != nil {
return ctrl.Result{}, err
}
log.Info("status updated")
}

if user.Spec.AuthType == cosmov1alpha1.UserAuthTypePasswordSecert {
// generate default password if password secret is not found
if _, err := password.GetDefaultPassword(ctx, r.Client, user.Name); err != nil && apierrors.IsNotFound(err) {
Expand All @@ -96,53 +90,59 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
}

// reconcile user addon
if len(user.Spec.Addons) > 0 {
errs := make([]error, 0)
addonErrs := make([]error, 0)

addonStatusMap := sliceToObjectMap(user.Status.Addons)
for _, addon := range user.Spec.Addons {
log.Info("syncing user addon", "addon", addon)
lastAddons := make([]cosmov1alpha1.ObjectRef, len(user.Status.Addons))
copy(lastAddons, user.Status.Addons)

inst := useraddon.EmptyInstanceObject(addon, user.GetName())
if inst == nil {
log.Info("WARNING: addon has no Template or ClusterTemplate", "addon", addon)
continue
}
currAddonsMap := make(map[types.UID]cosmov1alpha1.ObjectRef)
for _, addon := range user.Spec.Addons {
log.Info("syncing user addon", "addon", addon)

_, err := kubeutil.CreateOrUpdate(ctx, r.Client, inst, func() error {
return useraddon.PatchUserAddonInstanceAsDesired(inst, addon, user, r.Scheme)
})
if err != nil {
errs = append(errs, fmt.Errorf("failed to create or update addon %s :%w", inst.GetSpec().Template.Name, err))
continue
}
inst := useraddon.EmptyInstanceObject(addon, user.GetName())
if inst == nil {
log.Info("WARNING: addon has no Template or ClusterTemplate", "addon", addon)
continue
}

ct := inst.GetCreationTimestamp()
gvk, err := apiutil.GVKForObject(inst, r.Scheme)
if err != nil {
errs = append(errs, fmt.Errorf("failed to recognize addon instance GVK %s :%w", inst.GetSpec().Template.Name, err))
continue
}
addonStatusMap[inst.GetUID()] = cosmov1alpha1.ObjectRef{
ObjectReference: corev1.ObjectReference{
APIVersion: gvk.GroupVersion().String(),
Kind: gvk.Kind,
Name: inst.GetName(),
UID: inst.GetUID(),
ResourceVersion: inst.GetResourceVersion(),
},
CreationTimestamp: &ct,
}
op, err := kubeutil.CreateOrUpdate(ctx, r.Client, inst, func() error {
return useraddon.PatchUserAddonInstanceAsDesired(inst, addon, user, r.Scheme)
})
if err != nil {
addonErrs = append(addonErrs, fmt.Errorf("failed to create or update addon %s :%w", inst.GetSpec().Template.Name, err))
continue
}
user.Status.Addons = objectRefMapToSlice(addonStatusMap)

if len(errs) > 0 {
for _, e := range errs {
r.Recorder.Eventf(&user, corev1.EventTypeWarning, "AddonFailed", "failed to create or update user addon: %v", e)
log.Error(e, "failed to create or update user addon")
}
user.Status.Phase = "AddonFailed"
if op != controllerutil.OperationResultNone {
r.Recorder.Eventf(&user, corev1.EventTypeNormal, "Addon Synced", fmt.Sprintf("addon %s is %s", addon.Template.Name, op))
}

ct := inst.GetCreationTimestamp()
gvk, err := apiutil.GVKForObject(inst, r.Scheme)
if err != nil {
addonErrs = append(addonErrs, fmt.Errorf("failed to recognize addon instance GVK %s :%w", inst.GetSpec().Template.Name, err))
continue
}
currAddonsMap[inst.GetUID()] = cosmov1alpha1.ObjectRef{
ObjectReference: corev1.ObjectReference{
APIVersion: gvk.GroupVersion().String(),
Kind: gvk.Kind,
Name: inst.GetName(),
Namespace: inst.GetNamespace(),
UID: inst.GetUID(),
ResourceVersion: inst.GetResourceVersion(),
},
CreationTimestamp: &ct,
}
}
user.Status.Addons = objectRefMapToSlice(currAddonsMap)

if len(addonErrs) > 0 {
for _, e := range addonErrs {
r.Recorder.Eventf(&user, corev1.EventTypeWarning, "AddonFailed", "failed to create or update user addon: %v", e)
log.Error(e, "failed to create or update user addon")
}
user.Status.Phase = "AddonFailed"
}

// update user status
Expand All @@ -154,6 +154,28 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
log.Info("status updated")
}

// garbage collection
shouldDeletes := objectRefNotExistsInMap(lastAddons, currAddonsMap)
for _, d := range shouldDeletes {
log.Info("start garbage collection", "apiVersion", d.APIVersion, "kind", d.Kind, "name", d.Name)

var obj unstructured.Unstructured
obj.SetAPIVersion(d.APIVersion)
obj.SetKind(d.Kind)
err := r.Get(ctx, types.NamespacedName{Name: d.GetName(), Namespace: d.Namespace}, &obj)
if err != nil {
if !apierrs.IsNotFound(err) {
log.Error(err, "failed to get object to be deleted", "apiVersion", d.APIVersion, "kind", d.Kind, "name", d.Name)
}
continue
}

if err := r.Delete(ctx, &obj); err != nil {
r.Recorder.Eventf(&user, corev1.EventTypeWarning, "GCFailed", "failed to delete unused addon: %s %s", obj.GetKind(), obj.GetName())
}
r.Recorder.Eventf(&user, corev1.EventTypeNormal, "GC", "deleted unmanaged addon: %s %s", obj.GetKind(), obj.GetName())
}

log.Debug().Info("finish reconcile")
return ctrl.Result{}, err
}
Expand Down
Loading

0 comments on commit 748d334

Please sign in to comment.