Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

sys-apps/ignition: bring back noop OEM #2351

Merged
merged 2 commits into from
Jan 9, 2023
Merged

Conversation

tormath1
Copy link
Contributor

@tormath1 tormath1 commented Dec 21, 2022

it mainly brings back Vagrant which was failing with Ignition 2.14.0 even if no Ignition is provided.

Signed-off-by: Mathieu Tortuyaux mtortuyaux@microsoft.com


NOTE: It's ok to me to backport on all channels: the added platforms were not just working before.

Related to: flatcar/Flatcar#916

it mainly brings back Vagrant which was failing with Ignition 2.14.0
even if no Ignition is provided.

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1 tormath1 marked this pull request as ready for review January 3, 2023 08:13
@tormath1 tormath1 requested a review from a team January 3, 2023 08:13
Copy link
Contributor

@krnowak krnowak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this shouldn't be fixed by using noop.FetchConfig for the unregistered platforms, but I suppose this is outside the scope of this PR.

@tormath1
Copy link
Contributor Author

tormath1 commented Jan 3, 2023

I'm wondering if this shouldn't be fixed by using noop.FetchConfig for the unregistered platforms, but I suppose this is outside the scope of this PR.

What do you mean by that?

@krnowak
Copy link
Contributor

krnowak commented Jan 3, 2023

I'm wondering if this shouldn't be fixed by using noop.FetchConfig for the unregistered platforms, but I suppose this is outside the scope of this PR.

What do you mean by that?

Basically if somewhere in Ignition code there should be a snippet like (instead of trying to list all the old platforms):

if config, ok := configs.getConfigForPlatformOrSomething(platformName); ok {
    config.fetch(…)
} else {
    # platform not registered, assume noop.FetchConfig will be enough for its needs
    noop.FetchConfig(…)
}

But wrt. Ignition, I'm an uninformed keyboard puncher here, so I might be spouting nonsense. :)

@tormath1 tormath1 merged commit 379ca0a into main Jan 9, 2023
@tormath1 tormath1 deleted the tormath1/ignition-vagrant branch January 9, 2023 06:09
@tormath1
Copy link
Contributor Author

tormath1 commented Jan 9, 2023

@krnowak I'll go ahead for now - this is something we can suggest and track directly into the upstream coreos/ignition repo. :)

@tormath1
Copy link
Contributor Author

tormath1 commented Jan 9, 2023

picked to:

  • flatcar-3446
  • flatcar-3432

As it concerns Ignition, I think it's safer to test in Beta before picking it to stable.

@krnowak
Copy link
Contributor

krnowak commented Jan 9, 2023

@krnowak I'll go ahead for now - this is something we can suggest and track directly into the upstream coreos/ignition repo. :)

Sure, sounds good.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants