new butane variant: fedora-iot #487

say-paul · 2023-09-06T11:53:36Z

using the r4e config with fiot wrapper
Docs added to include the new variant

travier · 2023-09-06T14:02:25Z

I think you should add only the latest version as 1.0 and the experimental one as 1.1_exp.

travier · 2023-09-06T14:05:25Z

Why are you removing / renaming the R4E ones? You can not remove a config version once it's been released.

say-paul · 2023-09-06T17:03:18Z

Why are you removing / renaming the R4E ones? You can not remove a config version once it's been released.

Since we want butane to support both r4e and fedora-iot, so renaming the config path only for developer understanding. The end functionality and the variant names remains same. Do you see any other issue?

I think you should add only the latest version as 1.0 and the experimental one as 1.1_exp.

This is to make r4e and fiot in sync, so fiot_v1.0 being same as r4e_v1.1 f can create confusion and maintenance overhead. same for fiot_v1.1_exp -> r4e_v1.2_exp. Thoughts?

travier · 2023-09-06T17:51:19Z

If it's the same thing, why duplicate everything?

prestist · 2023-09-06T21:45:30Z

@say-paul A bit confused. As @travier said, we cannot remove an already released config, it would break anyone using it. Additionally, it sounds like the 'fiot' would be independent of rfe? while currently they have the same needs is there no eventuality where they fork to some degree?

say-paul · 2023-09-07T06:13:12Z

If it's the same thing, why duplicate everything?

@travier So idea is both rfe and fedora-iot user can use butane with their respective os variant. Since r4e is already there, this PR would simply add a fiot variant name with minimal code duplication.

As @travier said, we cannot remove an already released config, it would break anyone using it.

RegisterTranslator("r4e", "1.0.0", iot1_0.ToIgn3_3Bytes)
RegisterTranslator("r4e", "1.1.0", iot1_1.ToIgn3_4Bytes)
RegisterTranslator("r4e", "1.2.0-experimental", iot1_2_exp.ToIgn3_5Bytes)
RegisterTranslator("fiot", "1.0.0", iot1_0.ToIgn3_3Bytes)
RegisterTranslator("fiot", "1.1.0", iot1_1.ToIgn3_4Bytes)
RegisterTranslator("fiot", "1.2.0-experimental", iot1_2_exp.ToIgn3_5Bytes)

I understand this controls the variant name in the butane file which didn't change.

@travier @prestist Can you please clarify how it will break for existing user?

travier · 2023-09-07T08:09:04Z

It feels to me that R4E variants should use version numbers based on RHEL releases where the first version config started being supported while the IoT ones should keep the 1.x version numbers as we don't have a strong "no-rebase" policy in Fedora and we land new Ignition spec support there as we go.

travier · 2023-09-07T08:28:09Z

@travier So idea is both rfe and fedora-iot user can use butane with their respective os variant. Since r4e is already there, this PR would simply add a fiot variant name with minimal code duplication.

The lifecycle for RHEL 4 Edge and Fedora IoT do not match and the Ignition spec versions supported for a given Fedora IoT release tells nothing for RHEL 4 Edge and vice versa, so having the same numbers will not get us much and might instead create confusion.

say-paul · 2023-09-07T09:16:41Z

The lifecycle for RHEL 4 Edge and Fedora IoT do not match and the Ignition spec versions supported for a given Fedora IoT release tells nothing for RHEL 4 Edge and vice versa, so having the same numbers will not get us much and might instead create confusion.

Agreed, I will modify it accordingly.

travier · 2023-09-11T08:04:27Z

I've made #489 to track the R4E change.

say-paul · 2023-09-11T10:48:04Z

@travier made the changes as per the discussion.

config/fiot/v1_0/translate_test.go

docs/config-fiot-v1_0.md

travier · 2023-09-11T13:45:56Z

This is missing ~~all~~ some docs update. The table has to be updated, etc.

internal/doc/main.go

say-paul · 2023-09-11T14:57:13Z

This is missing ~~all~~ some docs update. The table has to be updated, etc.
Thanks for finding that out.

I think you are referring to the spec docs, updated them.

docs/specs.md

miabbott

LGTM

miabbott · 2023-10-03T12:54:41Z

@say-paul Are the CI errors still valid? Looks like they are just gofmt failures

cgwalters · 2023-10-03T14:07:52Z

Is there actually any "high level" sugar at all today in the r4e/fedora-iot versions?

There's only a little bit for fedora-coreos around things like boot raid etc.

I guess today we're disallowing partitioning in iot/edge?

I wonder if it'd make sense to introduce a generic e.g. "generic-systemd" style variant that just is YAML sugar for writing files and systemd units basically...

prestist

Sorry this has been sitting for so long, and thank you for going through the steps to add a new variant. I do have a small nit on your commit message. I would prefer it be more like =>
Add new fiot variant for fedora-iot

Otherwise it lgtm

say-paul · 2023-11-01T12:58:06Z

@prestist done.

miabbott · 2023-11-07T15:23:49Z

@prestist @travier Anything left to do in order to merge? I think all comments have been addressed at this point.

prestist · 2023-11-17T19:25:58Z

@say-paul @miabbott not from my perspective. @travier anything else ?

prestist · 2023-12-04T14:13:56Z

@say-paul

To get past the ci "Test 1.21.x ..." you need to rebase and -f push. The ci was changed external to this pr.

Otherwise from what I see this LGTM still.

v1_0 is compatible with ignition 3.4.0 v1_1_exp is experimental version compatible with ignition 3.5.0 docs added for fiot Signed-off-by: Sayan Paul <[email protected]>

say-paul · 2023-12-04T15:20:16Z

@prestist done

prestist · 2023-12-04T20:50:57Z

After speaking with @camgranella I am going to merge this as it lgtm!

say-paul force-pushed the butane_for_fedora-iot branch 2 times, most recently from fad3e8a to b13dc89 Compare September 6, 2023 13:58

say-paul force-pushed the butane_for_fedora-iot branch from 9fd761d to 5dbd7cb Compare September 11, 2023 10:45

7flying reviewed Sep 11, 2023

View reviewed changes

config/fiot/v1_0/translate_test.go Outdated Show resolved Hide resolved

7flying reviewed Sep 11, 2023

View reviewed changes

docs/config-fiot-v1_0.md Outdated Show resolved Hide resolved

say-paul commented Sep 11, 2023

View reviewed changes

internal/doc/main.go Show resolved Hide resolved

travier reviewed Sep 11, 2023

View reviewed changes

docs/specs.md Outdated Show resolved Hide resolved

say-paul force-pushed the butane_for_fedora-iot branch 2 times, most recently from eb576de to 9a45a28 Compare September 11, 2023 15:50

say-paul requested a review from travier September 12, 2023 07:23

miabbott approved these changes Oct 3, 2023

View reviewed changes

say-paul force-pushed the butane_for_fedora-iot branch 2 times, most recently from 590c508 to 1606137 Compare October 3, 2023 13:15

prestist approved these changes Oct 31, 2023

View reviewed changes

say-paul force-pushed the butane_for_fedora-iot branch from 1606137 to dd53201 Compare November 1, 2023 12:57

Add new fiot variant for fedora-iot

fed77f6

v1_0 is compatible with ignition 3.4.0 v1_1_exp is experimental version compatible with ignition 3.5.0 docs added for fiot Signed-off-by: Sayan Paul <[email protected]>

say-paul force-pushed the butane_for_fedora-iot branch from dd53201 to fed77f6 Compare December 4, 2023 14:38

prestist merged commit 25265e5 into coreos:main Dec 4, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new butane variant: fedora-iot #487

new butane variant: fedora-iot #487

say-paul commented Sep 6, 2023 •

edited

Loading

travier commented Sep 6, 2023

travier commented Sep 6, 2023

say-paul commented Sep 6, 2023 •

edited

Loading

travier commented Sep 6, 2023

prestist commented Sep 6, 2023

say-paul commented Sep 7, 2023

travier commented Sep 7, 2023

travier commented Sep 7, 2023

say-paul commented Sep 7, 2023

travier commented Sep 11, 2023

say-paul commented Sep 11, 2023

travier commented Sep 11, 2023 •

edited

Loading

say-paul commented Sep 11, 2023 •

edited

Loading

miabbott left a comment

miabbott commented Oct 3, 2023

cgwalters commented Oct 3, 2023

prestist left a comment •

edited

Loading

say-paul commented Nov 1, 2023

miabbott commented Nov 7, 2023

prestist commented Nov 17, 2023

prestist commented Dec 4, 2023

say-paul commented Dec 4, 2023

prestist commented Dec 4, 2023

new butane variant: fedora-iot #487

new butane variant: fedora-iot #487

Conversation

say-paul commented Sep 6, 2023 • edited Loading

travier commented Sep 6, 2023

travier commented Sep 6, 2023

say-paul commented Sep 6, 2023 • edited Loading

travier commented Sep 6, 2023

prestist commented Sep 6, 2023

say-paul commented Sep 7, 2023

travier commented Sep 7, 2023

travier commented Sep 7, 2023

say-paul commented Sep 7, 2023

travier commented Sep 11, 2023

say-paul commented Sep 11, 2023

travier commented Sep 11, 2023 • edited Loading

say-paul commented Sep 11, 2023 • edited Loading

miabbott left a comment

Choose a reason for hiding this comment

miabbott commented Oct 3, 2023

cgwalters commented Oct 3, 2023

prestist left a comment • edited Loading

Choose a reason for hiding this comment

say-paul commented Nov 1, 2023

miabbott commented Nov 7, 2023

prestist commented Nov 17, 2023

prestist commented Dec 4, 2023

say-paul commented Dec 4, 2023

prestist commented Dec 4, 2023

say-paul commented Sep 6, 2023 •

edited

Loading

say-paul commented Sep 6, 2023 •

edited

Loading

travier commented Sep 11, 2023 •

edited

Loading

say-paul commented Sep 11, 2023 •

edited

Loading

prestist left a comment •

edited

Loading