updating reconciler to understand how to delete resources in groups based on iac dependencies #815
Conversation
jhsinger-klotho
force-pushed
the
reconcile_iac
branch
from
1bd1def
to
5cfecb5
Compare
| namespacedResources, err := findAllResourcesInNamespace(c, resource) | ||
| if err != nil { | ||
| errs = errors.Join(errs, err) | ||
| continue | ||
| } |
There was a problem hiding this comment.
nit: move this closer to where it's used below
There was a problem hiding this comment.
we cant, ill add a comment, but it hast to be done before we remove the resource otherwise those resources wont be namespaced anymore
There was a problem hiding this comment.
The edges shouldn't impact it, since it looks like it just walks the graph. But for removing the resource, makes sense. In that case, can the for loop be moved up?
pkg/engine2/reconciler/reconciler.go
Outdated
| continue | ||
| } | ||
|
|
||
| for _, res := range namespacedResources.ToSlice() { |
There was a problem hiding this comment.
| for _, res := range namespacedResources.ToSlice() { | |
| for res := range namespacedResources { |
nit: don't need to copy the set to iterate it
pkg/engine2/reconciler/reconciler.go
Outdated
|
|
||
| for _, res := range namespacedResources.ToSlice() { | ||
|
|
||
| // Since we are explicitly deleting the namespace resource, we will explicitly delete the resources namespaced to it |
There was a problem hiding this comment.
nit: update comment, we're forwarding the explicit here, the comment suggests that it's passing in true
| found = true | ||
| } | ||
| default: | ||
| if reflect.ValueOf(val).Kind() == reflect.Slice || reflect.ValueOf(val).Kind() == reflect.Array { |
There was a problem hiding this comment.
This doesn't look deeper into maps, or lists of lists but that can be a TODO. Only case it might be relevant is for policies that have a PropertyRef.
There was a problem hiding this comment.
well that should be ok because doesnt loop properties get all indices of lists and keys of maps?
There was a problem hiding this comment.
You're right, but then shouldn't this not need to be added since it'd go into each index?
pkg/engine2/reconciler/reconciler.go
Outdated
| queue := make([]deleteRequest, 0) | ||
| queue = append(queue, deleteRequest{ | ||
| resource: resource, | ||
| explicit: explicit, | ||
| }) |
There was a problem hiding this comment.
| queue := make([]deleteRequest, 0) | |
| queue = append(queue, deleteRequest{ | |
| resource: resource, | |
| explicit: explicit, | |
| }) | |
| queue := []deleteRequest{{ | |
| resource: resource, | |
| explicit: explicit, | |
| }} |
nit: more concise
jhsinger-klotho
force-pushed
the
reconcile_iac
branch
from
5cfecb5
to
4db827f
Compare
jhsinger-klotho
force-pushed
the
reconcile_iac
branch
from
4db827f
to
ac10893
Compare
makes the reconciliation check iac dependencies of namespaced resources to see if we need to clean up more than just the namespaced resources
Standard checks