[node-agent] Introduce `gardener-node-init` script and unit #8726

rfranzke · 2023-10-29T18:01:07Z

How to categorize this PR?

/area dev-productivity
/kind enhancement

What this PR does / why we need it:
This PR introduces the bash script ("gardener-node-init") which will replace the cloud-config-downloader script when new shoot worker nodes are bootstrapped. It uses ctr images {pull,mount} to copy the gardener-node-agent binary from its container image to the node, and then runs gardener-node-agent bootstrap. This is a command which

(optional) format kubelet data volume (similar to how it's done today)
creates the gardener-node-agent.service systemd unit file (the "real" gardener-node-agent)
enables and starts this gardener-node-agent.service
disables and stops itself (the gardener-node-init.service)

From then on, gardener-node-agent takes care of reconciling the OSC secret with its OperatingSystemConfig controller (ref #8683). For self-upgrades, it may only restart itself at the very end of its reconciliation (after it persisted the "last applied OSC file") to prevent infinite loops.

Which issue(s) this PR fixes:
Part of #8023

Special notes for your reviewer:
On the way, I added a few more minor enhancements:

added missing client connection defaulting for GNA component config
extracted common run command initialisation code into reusable function

/cc @oliver-goetz

Release note:

NONE

ScheererJ

Thanks for the change, in particular the refactoring of the commands.

Initial feedback by @oliver-goetz and myself, 2 commits still to review.

pkg/nodeagent/bootstrap/bootstrap.go

pkg/nodeagent/controller/operatingsystemconfig/reconciler.go

ScheererJ

Finished review together with @oliver-goetz and found a few more things.

pkg/component/extensions/operatingsystemconfig/original/components/nodeagent/component.go

pkg/component/extensions/operatingsystemconfig/init/bootstrap_config.go

pkg/component/extensions/operatingsystemconfig/init/init.go

pkg/component/extensions/operatingsystemconfig/init/templates/scripts/init.tpl.sh

rfranzke · 2023-11-01T22:16:01Z

@ScheererJ @oliver-goetz Thanks for your reviews. I have addressed the feedback, and I have also added a few more things that I found while integrating this into gardenlet. In a nutshell:

both gardener-node-{init,agent} now gracefully shut down
file permissions adapted
ctr images pull and the containerd extractor should respect the hosts directory on the node to consider registry mirrors
containerd extractor needs to copy the file from an image into a temp file first and then rename it (direct copy leads to device or resource busy error)
kubeconfig was dropped and component config was enhanced with KAPI url + CA bundle (this simplifies the handling, especially for the migration from cloud-config-downloader later on

Please take another look and let me know what you think.

- client connection - servers - logging settings

Otherwise, GNA would restart itself too early, effectively triggering an infinite loop because it would always consider itself as "changed unit". We already followed the same approach in GNA's `node` controller. Co-Authored-By: Oliver Götz <47362717+oliver-goetz@users.noreply.github.com>

This code is the very same for almost each binary, so let's extract it into a function to not duplicate it again and again.

Co-Authored-By: Oliver Götz <47362717+oliver-goetz@users.noreply.github.com>

- OSC controller calls the root `context.CancelFunc` of the command - Bootstrap package no longer stops gardener-node-init unit - gardener-node-init unit no longer gets restarted each 30s but is a oneshot which only restarts on failure

Also add mutex to fake DBus to prevent flakes since the OSC controller in GNA is running systemd commands in parallel (and in the fake DBus they all write to the same slice in parallel)

Without this, registry mirrors in the hosts dir will not be respected. `ctr images pull` as well as the `docker.ConfigureHosts` function already ignore "not found" errors (in case the provided hosts dir does not exist), so we can safely specify it unconditionally

and then renames it - a direct copy while a process is running will not work (`device or resource busy`)

ScheererJ

Found a typo.

docs/extensions/operatingsystemconfig.md

This is much simpler compared to passing all potential arguments

ScheererJ

/lgtm

gardener-prow · 2023-11-03T11:51:03Z

LGTM label has been added.

Git tree hash: c80b97bc54a42a109d9ae1a648163fc94dc1a014

gardener-prow · 2023-11-03T12:30:10Z

@rfranzke: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gardener-apidiff	`a9d26a2`	link	false	`/test pull-gardener-apidiff`

Full PR test history. Your PR dashboard. Command help for this repository.
Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

oliver-goetz · 2023-11-03T13:18:44Z

awesome 🚀
/approve

gardener-prow · 2023-11-03T13:18:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oliver-goetz

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~~ [oliver-goetz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gardener-prow bot requested a review from oliver-goetz October 29, 2023 18:01

rfranzke mentioned this pull request Oct 29, 2023

☂️ Introduce Gardener Node Agent (a.k.a. replace cloud-config-downloader) #8023

Closed

55 tasks

rfranzke force-pushed the gna/node-init branch from acd875a to 21df46b Compare October 29, 2023 18:13

ScheererJ reviewed Oct 31, 2023

View reviewed changes

pkg/nodeagent/bootstrap/bootstrap.go Outdated Show resolved Hide resolved

pkg/nodeagent/controller/operatingsystemconfig/reconciler.go Outdated Show resolved Hide resolved

ScheererJ reviewed Oct 31, 2023

View reviewed changes

rfranzke force-pushed the gna/node-init branch from 21df46b to c828538 Compare November 1, 2023 22:15

rfranzke requested a review from ScheererJ November 1, 2023 22:16

rfranzke force-pushed the gna/node-init branch 2 times, most recently from ae1923a to ef86300 Compare November 2, 2023 08:05

rfranzke and others added 11 commits November 3, 2023 09:17

Add missing defaulting to GNA config

9fe1547

- client connection - servers - logging settings

Adapt UnitsToDiskScript to consider new .spec.units.files[] field

752b1ce

[prefactor] Extract common run command initialisation

c3c11d4

This code is the very same for almost each binary, so let's extract it into a function to not duplicate it again and again.

Introduce bootstrap command

68b1e0e

Co-Authored-By: Oliver Götz <47362717+oliver-goetz@users.noreply.github.com>

Introduce gardener-node-agent component in original OSC package

abbc767

Co-Authored-By: Oliver Götz <47362717+oliver-goetz@users.noreply.github.com>

Introduce gardener-node-init script and unit

0699b73

Co-Authored-By: Oliver Götz <47362717+oliver-goetz@users.noreply.github.com>

Address PR review feedback (graceful shutdown)

ef45484

- OSC controller calls the root `context.CancelFunc` of the command - Bootstrap package no longer stops gardener-node-init unit - gardener-node-init unit no longer gets restarted each 30s but is a oneshot which only restarts on failure

Address PR review feedback (nits)

b771a04

Also add mutex to fake DBus to prevent flakes since the OSC controller in GNA is running systemd commands in parallel (and in the fake DBus they all write to the same slice in parallel)

containerd extractor copies file to temp dir first

3261b43

and then renames it - a direct copy while a process is running will not work (`device or resource busy`)

ScheererJ requested changes Nov 3, 2023

View reviewed changes

docs/extensions/operatingsystemconfig.md Outdated Show resolved Hide resolved

gardener-prow bot assigned ScheererJ Nov 3, 2023

rfranzke force-pushed the gna/node-init branch from ef86300 to 5919a95 Compare November 3, 2023 08:59

rfranzke requested a review from ScheererJ November 3, 2023 08:59

rfranzke added 2 commits November 3, 2023 12:46

Address PR review feedback

389f0aa

Pass entire component config to node init

a9d26a2

This is much simpler compared to passing all potential arguments

rfranzke force-pushed the gna/node-init branch from 5919a95 to a9d26a2 Compare November 3, 2023 11:47

ScheererJ reviewed Nov 3, 2023

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2023

gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 3, 2023

gardener-prow bot merged commit 4e17e44 into gardener:master Nov 3, 2023
1 check passed

rfranzke deleted the gna/node-init branch November 3, 2023 13:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[node-agent] Introduce `gardener-node-init` script and unit #8726

[node-agent] Introduce `gardener-node-init` script and unit #8726

rfranzke commented Oct 29, 2023

ScheererJ left a comment

ScheererJ left a comment

rfranzke commented Nov 1, 2023

ScheererJ left a comment

ScheererJ left a comment

gardener-prow bot commented Nov 3, 2023

gardener-prow bot commented Nov 3, 2023 •

edited

Loading

oliver-goetz commented Nov 3, 2023

gardener-prow bot commented Nov 3, 2023

[node-agent] Introduce gardener-node-init script and unit #8726

[node-agent] Introduce gardener-node-init script and unit #8726

Conversation

rfranzke commented Oct 29, 2023

ScheererJ left a comment

Choose a reason for hiding this comment

ScheererJ left a comment

Choose a reason for hiding this comment

rfranzke commented Nov 1, 2023

ScheererJ left a comment

Choose a reason for hiding this comment

ScheererJ left a comment

Choose a reason for hiding this comment

gardener-prow bot commented Nov 3, 2023

gardener-prow bot commented Nov 3, 2023 • edited Loading

oliver-goetz commented Nov 3, 2023

gardener-prow bot commented Nov 3, 2023

[node-agent] Introduce `gardener-node-init` script and unit #8726

[node-agent] Introduce `gardener-node-init` script and unit #8726

gardener-prow bot commented Nov 3, 2023 •

edited

Loading