New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

internal/providers/system: support .d directory for base config fragments #1108

Merged

sohankunkerkar merged 3 commits into coreos:master from sohankunkerkar:config-dir

Oct 30, 2020

Member

sohankunkerkar commented Oct 9, 2020 •

edited

Loading

Fixes #1101
This change is inspired by coreos/fedora-coreos-config#626, which allows the two distros (i.e. RHCOS and FCOS) to carry different base.ign files with common elements. This will also support an additional .d directory for each platform ID.

We will need to make a few changes to the fedora-coreos-config repo especially here to split the base.ign config and move the afterburn-sshkeys into a separate fragment, and make corresponding changes to the module-setup.sh script.

sohankunkerkar force-pushed the config-dir branch from 0176286 to 9812246 Compare

October 9, 2020 03:18

sohankunkerkar mentioned this pull request

40ignition-conf: support .d dir for base config fragments coreos/fedora-coreos-config#676

Merged

Member

cgwalters commented Oct 13, 2020

I only skimmed this but the idea seems sane. I had a comment in coreos/fedora-coreos-config#676

sohankunkerkar force-pushed the config-dir branch 2 times, most recently from 31a4b21 to 891a265 Compare

October 15, 2020 21:49

arithx reviewed

View reviewed changes

internal/providers/system/system.go Outdated Show resolved Hide resolved

sohankunkerkar force-pushed the config-dir branch 6 times, most recently from d3d274f to 442a2ea Compare

October 23, 2020 03:50

bgilbert reviewed

View reviewed changes

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

sohankunkerkar force-pushed the config-dir branch 2 times, most recently from 62b6826 to 3d80c1f Compare

October 23, 2020 18:06

bgilbert reviewed

View reviewed changes

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated

+              	if err != nil {
+              		logger.Warning("no config at base.d: %v", err)
+              	}
+              	fullBaseConfig = latest.Merge(fullBaseConfig, baseDConfig)

Contributor

bgilbert Oct 23, 2020

Actually, we don't need to merge here; we can just fetch directly into fullBaseConfig.

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

sohankunkerkar force-pushed the config-dir branch 3 times, most recently from 022f0f3 to e3f8f8b Compare

October 23, 2020 20:15

Member Author

sohankunkerkar commented Oct 23, 2020

Local testing output:

[    4.989300] ignition[478]: reading system config file "/usr/lib/ignition/base.d/00-core.ign"
[    4.990706] ignition[478]: reading system config file "/usr/lib/ignition/base.d/30afterburn-sshkeys-core.ign"
[    4.992401] ignition[478]: no config dir at "/usr/lib/ignition/base.platform.d/qemu"

bgilbert reviewed

View reviewed changes

Contributor

bgilbert left a comment

Code looks good generally. tests/positive/general/preemption.go will need to be updated. We should also have a new automated test that checks correct merging of multiple base config and platform config fragments.

internal/providers/system/system.go Outdated Show resolved Hide resolved

sohankunkerkar force-pushed the config-dir branch from e3f8f8b to ff970fa Compare

October 26, 2020 17:46

bgilbert reviewed

View reviewed changes

tests/positive/general/preemption.go Outdated Show resolved Hide resolved

tests/positive/general/preemption.go Outdated Show resolved Hide resolved

tests/positive/general/preemption.go Outdated Show resolved Hide resolved

tests/positive/general/preemption.go Outdated Show resolved Hide resolved

tests/positive/general/general.go Outdated Show resolved Hide resolved

tests/positive/general/general.go Outdated Show resolved Hide resolved

tests/positive/general/general.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Outdated Show resolved Hide resolved

sohankunkerkar force-pushed the config-dir branch from ff970fa to 828fe11 Compare

October 26, 2020 22:57

sohankunkerkar changed the title ~~[WIP] internal/providers/system: support .d directory for base config fragments~~ internal/providers/system: support .d directory for base config fragments

arithx reviewed

View reviewed changes

internal/providers/system/system.go Outdated Show resolved Hide resolved

internal/providers/system/system.go Show resolved Hide resolved


          providers/system: support .d directory for base config fragments

e2d3fa3

Fixes coreos#1101
This change allows the two distrosi (i.e. FCOS and RHCOS) to carry
different base.ign files with common elements. Also,this will support
an additional .d directory for each platform ID.

bgilbert reviewed

View reviewed changes

tests/positive/general/baseconfigsmerging.go Show resolved Hide resolved

tests/positive/general/preemption.go Outdated Show resolved Hide resolved

sohankunkerkar force-pushed the config-dir branch 2 times, most recently from 1acaa71 to 06acba7 Compare

October 28, 2020 18:28

arithx approved these changes

View reviewed changes

Contributor

arithx left a comment

WFM

bgilbert reviewed

View reviewed changes

tests/positive/general/preemption.go Outdated Show resolved Hide resolved

sohankunkerkar added 2 commits

October 28, 2020 21:59


          tests/positive/general: update base config reference

533a7a4


          tests/positive/general: add test for merging multiple base configs

e5250ca

sohankunkerkar force-pushed the config-dir branch from 06acba7 to e5250ca Compare

October 29, 2020 01:59

bgilbert approved these changes

View reviewed changes

Contributor

bgilbert left a comment

🎉

sohankunkerkar mentioned this pull request

40ignition-conf: remove the legacy base config reference coreos/fedora-coreos-config#717

Merged

sohankunkerkar added a commit to coreos/fedora-coreos-config that referenced this pull request


          40ignition-conf: add new base config

da82417

Ignition now supports a `base.d` directory for base config fragments that allows FCOS and
RHCOS to maintain the same `00-core.ign`, and facilitate the distro-specific changes by placing
the other base config fragments in a separate `overlay` directory(coreos/ignition#1108).
This commit places 00-core.ign in a `base.d` directory.

Also, keeping the legacy base config and the Afterburn bit as-is, otherwise, it will break CI.
Once we have the new Ignition release, this PR (#717) will remove the legacy base config and revert #626

sohankunkerkar added a commit to coreos/fedora-coreos-config that referenced this pull request


          15fcos: place afterburn-sshkeys.ign in 50ignition-conf-fcos

b607e44

Make use of the new support for config fragments in Ignition (coreos/ignition#1108)
to move out the Afterburn bit added in #626
to a separate overlay directory so that RHCOS can more easily avoid it.

sohankunkerkar merged commit 43b5566 into coreos:master

sohankunkerkar deleted the config-dir branch

October 30, 2020 03:41

sohankunkerkar added a commit to sohankunkerkar/os that referenced this pull request


          overlay.d: remove 41rhcos-ignition-conf

af37af4

Ignition now supports a `base.d` directory for base config fragments that allows FCOS and
RHCOS to maintain the same `00-core.ign`, and facilitate the distro-specific changes by placing
the other base config fragments in a separate `overlay` directory(coreos/ignition#1108).

We don't need to maintain a separate base config for rhcos anymore.

sohankunkerkar mentioned this pull request

overlay.d: remove 41rhcos-ignition-conf openshift/os#432

Merged

sohankunkerkar added a commit to coreos/fedora-coreos-config that referenced this pull request


          40ignition-conf: remove legacy base config

f081f1c

Now the new release of Ignition has the support for base config fragments (coreos/ignition#1108),
we can safely remove the legacy base config and revert #626

sohankunkerkar added a commit to sohankunkerkar/os that referenced this pull request


          overlay.d: remove 41rhcos-ignition-conf

dd111c7

Ignition now supports a `base.d` directory for base config fragments that allows FCOS and
RHCOS to maintain the same `00-core.ign`, and facilitate the distro-specific changes by placing
the other base config fragments in a separate `overlay` directory(coreos/ignition#1108).

We don't need to maintain a separate base config for rhcos anymore.

sohankunkerkar added a commit to sohankunkerkar/os that referenced this pull request


          overlay.d: remove 41rhcos-ignition-conf

b84a484

Ignition now supports a `base.d` directory for base config fragments that allows FCOS and
RHCOS to maintain the same `00-core.ign`, and facilitate the distro-specific changes by placing
the other base config fragments in a separate `overlay` directory(coreos/ignition#1108).

We don't need to maintain a separate base config for rhcos anymore.

sohankunkerkar added a commit to sohankunkerkar/os that referenced this pull request


          submodule: Bump fedora-coreos-config

ba937af

Mostly for coreos/fedora-coreos-config#717 for coreos/ignition#1108

Shortlog with bot lockfile bumps omitted:

```
Kelvin Fan (4):
      coreos-copy-firstboot-network.sh: Place stamp file if copied
      30ignition-coreos: Add `coreos-boot-edit.{service,sh}`
      15coreos-network/coreos-copy-firstboot-network.sh: No stamp file
      05core: Merge `coreos-inject-rootmap.service` into `coreos-boot-edit.service`

Sohan Kunkerkar (2):
      overrides: fast-track Ignition 2.8.0
      40ignition-conf: remove legacy base config
```

sohankunkerkar added a commit to sohankunkerkar/os that referenced this pull request


          overlay.d: remove 41rhcos-ignition-conf

6f62afb

Ignition now supports a `base.d` directory for base config fragments that allows FCOS and
RHCOS to maintain the same `00-core.ign`, and facilitate the distro-specific changes by placing
the other base config fragments in a separate `overlay` directory(coreos/ignition#1108).

We don't need to maintain a separate base config for rhcos anymore.

kelvinfan001 pushed a commit to kelvinfan001/fedora-coreos-config that referenced this pull request


          40ignition-conf: add new base config

6d264c5

Ignition now supports a `base.d` directory for base config fragments that allows FCOS and
RHCOS to maintain the same `00-core.ign`, and facilitate the distro-specific changes by placing
the other base config fragments in a separate `overlay` directory(coreos/ignition#1108).
This commit places 00-core.ign in a `base.d` directory.

Also, keeping the legacy base config and the Afterburn bit as-is, otherwise, it will break CI.
Once we have the new Ignition release, this PR (coreos#717) will remove the legacy base config and revert coreos#626

kelvinfan001 pushed a commit to kelvinfan001/fedora-coreos-config that referenced this pull request


          15fcos: place afterburn-sshkeys.ign in 50ignition-conf-fcos

Make use of the new support for config fragments in Ignition (coreos/ignition#1108)
to move out the Afterburn bit added in coreos#626
to a separate overlay directory so that RHCOS can more easily avoid it.

kelvinfan001 pushed a commit to kelvinfan001/fedora-coreos-config that referenced this pull request


          40ignition-conf: remove legacy base config

6c4b095

Now the new release of Ignition has the support for base config fragments (coreos/ignition#1108),
we can safely remove the legacy base config and revert coreos#626

lucab mentioned this pull request

Automatic Network Configuration in Cloud with NetworkManager (nm-cloud-setup) coreos/fedora-coreos-tracker#320

Open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet