stages/ostree.deploy.container: add aleph file #10
Conversation
[skip ci]
lukewarmtemp
changed the title
Similar to the aleph file created for builds of FCOS based on ostree commit inputs, this adds an aleph file builds based on container image inputs.
lukewarmtemp
force-pushed
the
add-aleph-data
branch
from
c509abd
to
c45569a
Compare
This doesn't appear to include the Image ID which may or may not be something we want to carry forward, but was something that was there in the past: |
How could there be one in general? The point of this tooling is to produce disk images from container images. (It's not clear to me that retaining the image ID matters for the "coreos-using-imagebuilder" task) |
Similar to how we do it in create_disk today, we can do it here.
I'm mostly trying to look at what we have today and not lose fidelity. If someone does some analysis and tells me nothing is using this information, then sure, let's drop it. |
|
FYI, there are consumers of the aleph JSON right now. From a quick GitHub search:
Obviously, we could modify those since we own both of them and adapt them to work with both pre-osbuild and post-osbuild images. But it's way simpler to just keep the fields in there and augment with more fields as relevant. |
|
In the bootupd case though it's not really relevant going forward because there shouldn't be any non-bootupd cases that we ship. (And the non-coreos, non-bootupd case of atomic desktops today doesn't use the aleph either) So it's really just the MCO which is still just informational that we can easily adapt. |
|
Had a quick session with the team to try to gain some traction on this for the time being:
|
|
@cgwalters for This is currently being grabbed from the image with |
For ease of introspection; xref e.g. dustymabe/osbuild#10
|
It's part of the ostree commit metadata. I did ostreedev/ostree-rs-ext#560 to make this easier to extract from other languages. |
If we can reasonably drop the |
the container image metadata
lukewarmtemp
force-pushed
the
add-aleph-data
branch
from
ab0ba9c
to
f0bfc5f
Compare
|
@dustymabe @cgwalters @jlebon Changes made to the contents of the aleph file, please see the modified top comment of the PR for the updated values of
|
lukewarmtemp
force-pushed
the
add-aleph-data
branch
from
f0bfc5f
to
30f4a58
Compare
lukewarmtemp
force-pushed
the
add-aleph-data
branch
from
30f4a58
to
0270cc2
Compare
Fixes some of the labels in the aleph file. Calling `ostree container image metadata` also requires the target image reference to be only the container image and tag (ie. without the signature verification type and remote).
lukewarmtemp
force-pushed
the
add-aleph-data
branch
from
0270cc2
to
70a98b1
Compare
| encoding="utf8", | ||
| stdout=sys.stderr, | ||
| stdout=subprocess.PIPE, |
There was a problem hiding this comment.
ahh. I hadn't realized we'd need to make this change.. I honestly think this is the way it should be, but the osbuild devs might want stdout to still go to stderr to keep the same behavior. We can still achieve that goal if they ask us by just doing:
cp = subprocess.run(["ostree"] + args,
encoding="utf8",
stdout=sys.stderr,
stdout=subprocess.PIPE,\
input=_input,
check=True)
print(cp.stdout)
return cp
but let's wait and see what they say.
lukewarmtemp
force-pushed
the
add-aleph-data
branch
3 times, most recently
from
4783693
to
706dfee
Compare
lukewarmtemp
force-pushed
the
add-aleph-data
branch
from
706dfee
to
a89ea3b
Compare
lukewarmtemp
force-pushed
the
add-aleph-data
branch
from
a89ea3b
to
e14c1cc
Compare
|
This is looking pretty good to me. @jlebon - want to give it another round? |
| @@ -215,15 +214,24 @@ def parse_input_commits(commits): | |||
| def deployment_path(root: PathLike, osname: str, ref: str, serial: int): | |||
| """Return the path to a deployment given the parameters""" | |||
|
|
|||
| base = os.path.join(root, "ostree") | |||
| if osname == "" and ref == "" and serial == None: | |||
There was a problem hiding this comment.
Is there even a use case in osbuild for having multiple deployments in the image? I'd just nuke those parameters and always do the glob approach. That should simplify some of the callers currently doing extra work to prepare those params.
Obviously fine to do as a follow-up.
There was a problem hiding this comment.
yeah, I think there is an argument to make for that - +1 for a follow up. We'd need to keep supporting specifying the options even though they would do nothing.
There was a problem hiding this comment.
If we're making the main aleph file non-CoreOS specific, then we should probably not have "coreos" in its name.
This commit makes the code more clear by refactoring the part extracts the deployment type and imgref from the origin file into a separate function. This function was implemented as an ostree tool such that other stages can also utilize it. Several other minor changes were also made to clean up the code
|
upstream PR opened (with some tweaks): osbuild#1475 |
This commit removes the dependency of the imgid field in the coreos aleph file. As per the discussion in the following PR to OSBuild, as decision was made to remove the field: dustymabe/osbuild#10 (comment) A decision was also made to log the entire aleph file since it is more verbose.
This commit removes the dependency of the imgid field in the coreos aleph file. As per the discussion in the following PR to OSBuild, as decision was made to remove the field: dustymabe/osbuild#10 (comment) A decision was also made to log the entire aleph file since it is more verbose.
Similar to the aleph file created for builds of FCOS based on ostree commit inputs, this adds an aleph file for builds based on container image inputs.