prune tenant resources #1458
prune tenant resources #1458
Conversation
alichaddad
requested review from
bigkevmcd,
sarataha and
jmickey
and removed request for
bigkevmcd and
sarataha
pkg/tenancy/tenancy.go
Outdated
| @@ -104,27 +103,99 @@ func (t Tenant) Validate() error { | |||
| } | |||
|
|
|||
| // CreateTenants creates resources for tenants given a file for definition. | |||
| func CreateTenants(ctx context.Context, config *Config, c client.Client, out io.Writer) error { | |||
| resources, err := GenerateTenantResources(config) | |||
| func CreateTenants(ctx context.Context, config *Config, c client.Client, prune bool, out io.Writer) error { | |||
There was a problem hiding this comment.
Maybe we can update this to reflect what it does now?
It's now applying the Tenant resources.
It also doesn't handle deletion of removed tenants?
We can use client.HasLabels ?
pkg/tenancy/tenancy.go
Outdated
| if prune { | ||
| err := kubeClient.Delete(ctx, existingResource) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to clean up tenant %s resources: %w", tenant.Name, err) |
There was a problem hiding this comment.
Should this be fatal?
Given how far into the process it could be, would it be better to output a message to say that it failed to delete a resource, and continue?
What do existing tools do in this case?
pkg/tenancy/tenancy.go
Outdated
|
|
||
| // cleanResources cleanup resources that should be pruned when certain configuration is removed | ||
| func cleanResources(ctx context.Context, tenant Tenant, kubeClient client.Client, newResources, existingResources []client.Object, prune bool, out io.Writer) error { | ||
| newResourcesMap := make(map[string]client.Object) |
There was a problem hiding this comment.
I think we can make this code easier to understand, and removing the existing resources for delete tenants is part of that.
If you have a set of resources to create and a set of existing resources, you can calculate the set of resources that you need to create, the set of resources you need to update, and the set of resources you need to delete.
alichaddad
force-pushed
the
1252_prune_resources
branch
from
9813af7 to
9ca9eaf
Compare
| // upsert applies runtime objects to the cluster, if they already exist, | ||
| // patching them with type specific elements. | ||
| func upsert(ctx context.Context, kubeClient client.Client, obj client.Object, out io.Writer) error { | ||
| existing := runtimeObjectFromObject(obj) | ||
| objectID := fmt.Sprintf("%s/%s", strings.ToLower(obj.GetObjectKind().GroupVersionKind().GroupKind().String()), obj.GetName()) | ||
| objectID := getObjectID(obj) |
| } | ||
|
|
||
| // getResourcesToDelete returns list of resources to be deleted from resources to be generated and existing resources | ||
| func getResourcesToDelete(newResources, existingResources []client.Object) []client.Object { |
There was a problem hiding this comment.
Clean 👍 , I'm coming around to golang being all verbose and explicit. If this gets more complicated and we wanna add another layer, https://github.com/kubernetes/apimachinery/blob/master/pkg/util/sets/string.go has been popping up in the core codebase which can be slick for stuff like this.
resourcesToDelete = existingResources.difference(newResources)
alichaddad
force-pushed
the
1252_prune_resources
branch
from
9ca9eaf to
631b589
Compare
No description provided.