Repository navigation
kinder: add native workflow groups and an extended test suite - #3334
HirazawaUi wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: HirazawaUi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
cncf-cla: yes
|
/assign @neolit123 |
There was a problem hiding this comment.
i did a first pass. we should have likely had more discussion on the ticket before the PR. main point is about these nested / extended workflows. do we really want these?
i am all about adding an etcd recovery test, but we have to keep in mind that if we add a new FG that stops doing the auto recovery this test must change. i.e. the same way the user would have to manually recover etcd from an original etcd snapshot.
i did not spent a lot of time on the context/cancelation/timeout improvements. i don't understand them quite well, but generally adding such improvements and unit tests around workflows are fine, as long as we understand the logic properly as humans...
| workflows: | ||
| - name: upgrade-etcd-rollback | ||
| workflow: upgrade-etcd-rollback-{{ .KubernetesVersion }}.yaml | ||
| timeout: 70m | ||
| - name: upgrade-no-addon-config-maps | ||
| workflow: upgrade-no-addon-config-maps-{{ .KubernetesVersion }}.yaml | ||
| timeout: 70m |
There was a problem hiding this comment.
do we really want to nest workflows within workflows? the mapping of one prow job -> one kinder workflow has been flexible enough for us. sure it can be annoying to have to update these in 2 repositories, but we have a generator for that purpose.
TBH, i am not so sure about this. additionally it introduces running upgrade-no-addon-config-maps in a second location, which duplicates testing the same aspect.
could we simply introduce the dedicated upgrade-etcd-rollback workflow + its tasks?
| # Kubeadm installs this patch after backup. Kubelet terminates the original | ||
| # static Pod before starting its replacement, so the init container owns the | ||
| # data directory exclusively. Preserve all DB/WAL bytes rather than truncating | ||
| # a database file; the regular file at "member" makes old etcd require restore. | ||
| cat > "${state}/patches/etcd+strategic.yaml" <<'PATCH' |
There was a problem hiding this comment.
could we avoid an init container and static pod patches?
i was wondering if we could just corrupt the member date before uptrade from a task (not from an init container) and just execute kubeadm upgrade from the built-in kinder subcommand?
i am trying to avoid complicated bash in kinder, although we do have a few tasks with complicated bash already...
if we have to do some complicated state, maybe we could do it from kinder's go code...
| - name: collect-rollback-diagnostics | ||
| cmd: docker | ||
| args: | ||
| - exec | ||
| - '{{ .vars.clusterName }}-control-plane-1' | ||
| - bash | ||
| - -ceu | ||
| - | | ||
| mkdir -p /kinder/rollback | ||
| find /var/lib/etcd -maxdepth 6 -type f -printf '%p %s bytes\n' > /kinder/rollback/data-files.txt | ||
| find /var/lib/etcd -maxdepth 1 -printf '%f %y\n' > /kinder/rollback/data-entries.txt | ||
| crictl ps -a > /kinder/rollback/containers.txt | ||
| force: true | ||
| ignoreError: true | ||
| - name: export-rollback-diagnostics | ||
| cmd: docker | ||
| args: | ||
| - cp | ||
| - '{{ .vars.clusterName }}-control-plane-1:/kinder/rollback' | ||
| - '{{ .env.ARTIFACTS }}/etcd-rollback' | ||
| force: true | ||
| ignoreError: true |
There was a problem hiding this comment.
i am not so sure why these are two separate tasks. also do we really care about diagonostics?
if the rollback passed maybe we can just end the workflow.
| @@ -0,0 +1,551 @@ | |||
| /* | |||
There was a problem hiding this comment.
there is a lot in this file.
let's discuss first if we want to support these nested workflows.
i am generally +1 to have more unit tests related to workflows, but i would prefer if they are in workflow_test.go.
| // Run executes a workflow or scenario group. One cancellation context covers the | ||
| // entire tree, including the intervals between commands and child workflows. | ||
| func (w *Workflow) Run(out io.Writer, dryRun, verbose, exitOnError bool, artifacts string) error { |
There was a problem hiding this comment.
i think all this can go in workflow.go. the file is not that big and all methods should ideally live where the struct definition is (unless the file is huge, which it isn't).
Related to #3331.
Add native workflow groups so a shared Kinder entry point can run independent scenarios while preserving their individual execution state and results.
Child workflows execute sequentially in the same process, with isolated variables, artifacts, and JUnit reports. Failed or timed-out scenarios allow subsequent scenarios to run; cancellation stops new scenarios while preserving forced cleanup and reporting. Existing task workflows and imports remain supported.
This change also adds recursive configuration validation, cycle detection, scenario deadlines, and an extended-latest suite combining an etcd rollback regression scenario with the existing missing addon ConfigMap scenario.
AI Usage Disclosure: I came up with the idea, and gpt-6 completed the code implementation.