Skip to content

Mantle: use Butane libs#2243

Closed
darkmuggle wants to merge 3 commits into
coreos:mainfrom
darkmuggle:pr/butane
Closed

Mantle: use Butane libs#2243
darkmuggle wants to merge 3 commits into
coreos:mainfrom
darkmuggle:pr/butane

Conversation

@darkmuggle
Copy link
Copy Markdown
Contributor

Changes:

  • vendor in Butane
  • added helper function
  • change qemuexec to use Butane libs over FCCT

While looking at some tests, I noticed a pattern of:

	/* The FCCT config used for generating this ignition config:
	variant: fcos
	version: 1.3.0
	boot_device:
	  mirror:
	    devices:
	      - /dev/sda
	      - /dev/sdb
	      - /dev/sdc
	*/
	bootmirror = <long json>

This change will allow developers the option to use YAML through Butane or write the long JSON.

@cgwalters
Copy link
Copy Markdown
Member

cgwalters commented Jun 23, 2021

Did you have a specific reason to switch to vendoring vs forking the binary? (Commit message doesn't say) Currently we don't have dependabot set up on this repo, which is mostly OK but butane is something we will care about and want to keep updated. Right now we have implicit updates when the Fedora package is built. (Also, because not all tooling in this repository is in Go, we need to keep butane as a binary anyways)

But I'm OK with this too!

@bgilbert
Copy link
Copy Markdown
Contributor

bgilbert commented Jun 23, 2021

👍, APIs >> subprocesses. We can add "update Mantle vendoring" to the Butane release checklist, as we currently have with Ignition.

But let's properly integrate this into platform/conf, rather than adding a box-on-the-side internal API. Let me dig into this a bit, it shouldn't be hard to do.

@cgwalters
Copy link
Copy Markdown
Member

+1, APIs >> subprocesses.

Yes, as a general rule. But I think my point stands that this specific repository is gluing together a lot of different tools in different languages - e.g. we also fork rpm-ostree and parse its JSON, that's unlikely to change. And I definitely run butane on the CLI too, so it will be surprising if the versions differ.

(Anyways not arguing against this either to be clear)

@bgilbert
Copy link
Copy Markdown
Contributor

#2246 integrates Butane support into platform/conf and updates the affected tests.

@darkmuggle
Copy link
Copy Markdown
Contributor Author

Superseded by #2246, which is a superior solution. Closing.

@darkmuggle darkmuggle closed this Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants