chore: Change solver to decide which tasks to use

kubernetes-sigs / cli-utils

This repo contains binaries that built from libraries in cli-runtime.

Apache License 2.0

154 stars 77 forks source link

chore: Change solver to decide which tasks to use #549

Closed karlkfi closed 2 years ago

karlkfi commented 2 years ago

This redesign of the solver moves complexity about deciding which tasks to use from the applier/destroyer into the solver. This makes the solver more of a solver and less of a "do what your told"-er.
One major difference is that the InvAddTask is now skipped if there are no objects to be applied. This makes logic sense because the inventory doesn't need to be expanded.
This change unblocks unification of graph dependency resolution using both the apploy objects and prune objects, which will help distinguish between external dependencies and dependencies being accidentally deleted.

karlkfi commented 2 years ago

/retest

karlkfi commented 2 years ago

These example tests seem to be erroring over a non-error...

deployment.apps/the-deployment deleted
service/the-service deleted
configmap/the-map2 deleted
3 resource(s) deleted, 0 skipped, 0 failed to delete
Success: output line found
Success: output line found
Success: output line found
Preview strategy: server
Error: output line not found
Expected: deployment.apps/the-deployment deleted]

UPDATE: nvm. the client-side preview works, but server-side preview doesn't print any of the expected delete lines.

k8s-ci-robot commented 2 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karlkfi, mortent

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/cli-utils/blob/master/OWNERS)~~ [mortent] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

karlkfi commented 2 years ago

Found the cause of the test failure.

The DryRunStrategy was being passed in both the Options and the Builder structs. So it wasn't be propagated consistently. I've removed the one in the Builder and left it only in the Options.

karlkfi commented 2 years ago

/test cli-utils-presubmit-master