Adapt to changes needed for `gardener-node-agent` #130

rfranzke · 2023-10-13T08:29:02Z

How to categorize this PR?

/area os
/kind enhancement
/os garden-linux

What this PR does / why we need it:
This PR adapts this extension to changes needed for gardener-node-agent, see gardener/gardener#8647 for more information.

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

Special notes for your reviewer:

Depends on Prepare OperatingSystemConfig API for changes needed for gardener-node-agent gardener#8647 and a new release containing this PR, hence this PR remains in draft state until then.

Kindly use the dedicated commits for an easier review process.

/cc @oliver-goetz @MrBatschner @danielfoehrKn

Release note:

This extension is now prepared to run with an enabled `UseGardenerNodeAgent` feature gate.

rfranzke · 2023-10-20T14:19:34Z

PR is ready for review now that g/g@v1.82 is released

rfranzke · 2023-10-20T14:21:00Z

I plan to add a few unit tests for the new actuator.go file early next week.

rfranzke · 2023-10-25T12:12:12Z

OK, @oliver-goetz @MrBatschner @danielfoehrKn PTAL :)

rfranzke · 2023-10-27T14:58:35Z

/hold
Let's wait with the merge for another revendoring with gardener/gardener@v1.83.0 containing gardener/gardener#8707

- no longer use `oscommon` package - prepare for new OSC contract when `UseGardenerNodeAgent` feature gate is enabled

- no longer depend on `oscommon` package

pkg/controller/operatingsystemconfig/actuator.go

MrBatschner

see inline

MrBatschner · 2023-11-24T11:30:21Z

pkg/controller/operatingsystemconfig/generator/generator.go

-	return osc.Object.Spec.Type == gardenlinux.OSTypeGardenLinux ||
-		osc.Object.Spec.Type == memoryone.OSTypeMemoryOneGardenLinux
-}
-
 // Generate generates a Garden Linux specific cloud-init script from the given OperatingSystemConfig.
 func (g *GardenLinuxCloudInitGenerator) Generate(logger logr.Logger, osc *generator.OperatingSystemConfig) ([]byte, *string, error) {


Maybe I am not fully understanding the flow and/or intention but in

gardener-extension-os-gardenlinux/pkg/controller/operatingsystemconfig/actuator.go

Line 88 in e9ab9ee

func (a *actuator) handleProvisionOSC(ctx context.Context, osc *extensionsv1alpha1.OperatingSystemConfig) (string, error) {

, the data for CloudInit is generated by the handleProvisionOSC() function, yet here, if useGardenerNodeAgent is false, the old template based generator is used. From what I can see, the actual output seems to be the same for both generating paths.
Can't we do away with the templater and use handleProvisionOSC() and handleReconcileOSC() exclusively for both cases?

Maybe we could, but I would prefer to keep things separate for now so that the old legacy generator can be easily deleted later when gardener-node-agent is GA and the cloud-config-downloader scenario fully disappeared. This way, we don't change anything for the current world (no gardener-node-agent in place), but just for the new feature when the feature gate is turned on. Hence, if anything goes wrong, it's just for the new scenario, not for the old one.

We still need to run the generator anyways for the migration scenario (when the UseGardenerNodeAgent feature gate is turned on for the first time) where cloud-config-downloader still runs on existing nodes. It needs to download the new user data (containing the gardener-node-agent systemd unit), so we wouldn't gain much from trying to harmonize anything now here.

danielfoehrKn

Need to rework #133 after this PR is merged.
Looks good to me. I like that the gardener-node-agent simplifies the extensions even more (generation of the execution script with systemd units & files is done not in each extension, but on the node via the gardener-node-agent)

pkg/controller/operatingsystemconfig/actuator.go

MrBatschner

/lgtm

rfranzke · 2023-11-27T09:23:52Z

Can we release this PR?

gardener-robot added area/os Operation system related kind/enhancement Enhancement, improvement, extension os/garden-linux Related to Garden Linux OS needs/review Needs review labels Oct 13, 2023

rfranzke mentioned this pull request Oct 13, 2023

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

Closed

55 tasks

gardener-robot added size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Oct 13, 2023

rfranzke force-pushed the gna branch from 0331e23 to 33630fa Compare October 20, 2023 14:18

gardener-robot added size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) and removed size/l Size of pull request is large (see gardener-robot robot/bots/size.py) labels Oct 20, 2023

rfranzke marked this pull request as ready for review October 20, 2023 14:19

rfranzke requested review from a team as code owners October 20, 2023 14:19

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 20, 2023

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 20, 2023

rfranzke force-pushed the gna branch from 33630fa to 8e47f73 Compare October 25, 2023 12:12

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 25, 2023

rfranzke force-pushed the gna branch from 8e47f73 to 4c58281 Compare October 25, 2023 12:13

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2023

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2023

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 26, 2023

rfranzke added 4 commits November 17, 2023 15:37

Introduce OSC controller

aa709d2

- no longer use `oscommon` package - prepare for new OSC contract when `UseGardenerNodeAgent` feature gate is enabled

Rework main entrypoint

be4fced

- no longer depend on `oscommon` package

[make revendor]

490ca4e

Set UseGardenerNodeAgent option based on gardenlet feature gate

e9ab9ee

rfranzke force-pushed the gna branch from 494c248 to e9ab9ee Compare November 17, 2023 14:37

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 17, 2023

MrBatschner reviewed Nov 24, 2023

View reviewed changes

pkg/controller/operatingsystemconfig/actuator.go Outdated Show resolved Hide resolved

MrBatschner reviewed Nov 24, 2023

View reviewed changes

Address PR review feedback

2c72840

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 24, 2023

rfranzke requested a review from MrBatschner November 24, 2023 12:11

danielfoehrKn reviewed Nov 24, 2023

View reviewed changes

pkg/controller/operatingsystemconfig/actuator.go Show resolved Hide resolved

Address PR review feedback

710b423

rfranzke requested a review from danielfoehrKn November 24, 2023 13:29

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 24, 2023

danielfoehrKn approved these changes Nov 24, 2023

View reviewed changes

MrBatschner approved these changes Nov 24, 2023

View reviewed changes

gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review needs/second-opinion Needs second review by someone else labels Nov 24, 2023

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 24, 2023

danielfoehrKn merged commit b42ab45 into gardener:master Nov 24, 2023
5 checks passed

gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Nov 24, 2023

rfranzke deleted the gna branch November 27, 2023 08:10

rfranzke mentioned this pull request Feb 23, 2024

Set --gardenlet-uses-gardener-node-agent=true by default #148

Merged

rfranzke mentioned this pull request Apr 10, 2024

Cleanup legacy code for cloud-config-downloader scenario #161

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt to changes needed for `gardener-node-agent` #130

Adapt to changes needed for `gardener-node-agent` #130

rfranzke commented Oct 13, 2023 •

edited

Loading

rfranzke commented Oct 20, 2023

rfranzke commented Oct 20, 2023

rfranzke commented Oct 25, 2023

rfranzke commented Oct 27, 2023

MrBatschner left a comment

MrBatschner Nov 24, 2023 •

edited

Loading

rfranzke Nov 24, 2023

danielfoehrKn left a comment

MrBatschner left a comment

rfranzke commented Nov 27, 2023 •

edited

Loading

Adapt to changes needed for gardener-node-agent #130

Adapt to changes needed for gardener-node-agent #130

Conversation

rfranzke commented Oct 13, 2023 • edited Loading

rfranzke commented Oct 20, 2023

rfranzke commented Oct 20, 2023

rfranzke commented Oct 25, 2023

rfranzke commented Oct 27, 2023

MrBatschner left a comment

Choose a reason for hiding this comment

MrBatschner Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

rfranzke Nov 24, 2023

Choose a reason for hiding this comment

danielfoehrKn left a comment

Choose a reason for hiding this comment

MrBatschner left a comment

Choose a reason for hiding this comment

rfranzke commented Nov 27, 2023 • edited Loading

Adapt to changes needed for `gardener-node-agent` #130

Adapt to changes needed for `gardener-node-agent` #130

rfranzke commented Oct 13, 2023 •

edited

Loading

MrBatschner Nov 24, 2023 •

edited

Loading

rfranzke commented Nov 27, 2023 •

edited

Loading