Skip to content

Conversation

@cgwalters
Copy link
Member

The current mantle/cosa directory is a bit circular, we
basically have cosa/mantle/cosa then. It made sense before
the projects were merged but less so now I think.

Second, the mantle/cosa/builds.go is really about the meta.json
file but we also need the directory for a build. What's
going on in kola is quite weird because we basically need
to keep around the pair of (directory, parsed meta).

Enhance the mantle/sdk to learn about a LocalBuild struct
and teach kola how to use it to synthesize the other two if
. looks like a coreos-assembler root.

This drains more logic from cmd-kola in cosa, trending
towards removing it!

@cgwalters
Copy link
Member Author

Copy link
Contributor

@arithx arithx left a comment

Choose a reason for hiding this comment

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

Last commit makes sense to me.

@cgwalters
Copy link
Member Author

A follow on to this per the first para of git commit msg is to git mv mantle/cosa/builds.go mantle/sdk/cosameta.go or so if agreed w/the direction /cc @darkmuggle

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Yup, makes sense to me overall to start making mantle more cosa-aware.

@jlebon
Copy link
Member

jlebon commented Mar 20, 2020

/lgtm

@cgwalters
Copy link
Member Author

The CI run here is stuck in emergency mode...looks like we have an old coreos-installer for some reason?

And we really need something like coreos/ignition-dracut#146 so we can consistently detect this and error out fast.

@bgilbert
Copy link
Contributor

looks like we have an old coreos-installer for some reason?

Error: image fetch failed: 404 Not Found

Looks like a test flake.

@cgwalters
Copy link
Member Author

Hmm...I don't think this should be racy (though it's possible). Without trying to reproduce locally my current guess is this has something to do with us implicitly changing from absolute paths to relative .. I think we want to use absolute to be resilient to chdir changes, so updated to do that.

@jlebon
Copy link
Member

jlebon commented Mar 23, 2020

/lgtm

@cgwalters
Copy link
Member Author

/hold
debugging this locally

@cgwalters
Copy link
Member Author

OK I logged into the pod live and:

sh-5.0# ls -al /tmp/kola-testiso728028448/tftp/
total 164
drwxr-xr-x. 3 builder builder    263 Mar 23 18:20 .
drwx------. 3 builder builder     18 Mar 23 18:20 ..
-rw-r--r--. 1 builder builder    549 Mar 23 18:20 config.ign
lrwxrwxrwx. 1 builder builder     83 Mar 23 18:20 fedora-coreos-31.20200323.dev.0-live-initramfs.x86_64.img -> /srv/builds/latest/x86_64/fedora-coreos-31.20200323.dev.0-live-initramfs.x86_64.img
lrwxrwxrwx. 1 builder builder     76 Mar 23 18:20 fedora-coreos-31.20200323.dev.0-live-kernel-x86_64 -> /srv/builds/latest/x86_64/fedora-coreos-31.20200323.dev.0-live-kernel-x86_64
lrwxrwxrwx. 1 builder builder     77 Mar 23 18:20 fedora-coreos-31.20200323.dev.0-metal.x86_64.raw.gz -> /srv/builds/latest/x86_64/fedora-coreos-31.20200323.dev.0-metal.x86_64.raw.gz
-rw-r--r--. 1 builder builder 116320 Jul 27  2019 ldlinux.c32
-rw-r--r--. 1 builder builder  42995 Jul 27  2019 pxelinux.0
drwxr-xr-x. 2 builder builder     21 Mar 23 18:20 pxelinux.cfg
sh-5.0# ls -al /srv/builds/latest/x86_64/
total 648008
drwxr-xr-x. 2 builder builder       254 Mar 23 18:22 .
drwxr-xr-x. 3 builder builder        20 Mar 23 18:22 ..
-r--r--r--. 1 builder builder     33962 Mar 23 18:22 commitmeta.json
-r--r--r--. 1 builder builder       493 Mar 23 18:21 coreos-assembler-config-git.json
-r--r--r--. 1 builder builder     29541 Mar 23 18:21 coreos-assembler-config.tar.gz
-r--r--r--. 1 builder builder 663439360 Mar 23 18:05 fedora-coreos-31.20200323.dev.0-4-ostree.x86_64.tar
-r--r--r--. 1 builder builder     23847 Mar 23 18:22 manifest-lock.generated.x86_64.json
-rw-r--r--. 1 builder builder      1948 Mar 23 18:22 meta.json
-r--r--r--. 1 builder builder     15580 Mar 23 18:22 ostree-commit-object
sh-5.0# 

Somehow we're missing everything except the ostree?

@cgwalters
Copy link
Member Author

Heh, from the clitests:

+ cosa prune --keep-last-n=1

Pruning build 31.20200323.dev.0-3

Pruning build 31.20200323.dev.0-2

Pruning build 31.20200323.dev.0-1

So that's definitely racing.

The current `mantle/cosa` directory is a bit circular, we
basically have `cosa/mantle/cosa` then.  It made sense before
the projects were merged but less so now I think.

Second, the `mantle/cosa/builds.go` is really about the `meta.json`
file but we also need the *directory* for a build.  What's
going on in kola is quite weird because we basically need
to keep around the pair of (directory, parsed meta).

Enhance the `mantle/sdk` to learn about a `LocalBuild` struct
and teach kola how to use it to synthesize the other two if
`.` looks like a coreos-assembler root.

This drains more logic from `cmd-kola` in cosa, trending
towards removing it!
@cgwalters
Copy link
Member Author

And hooray, passing tests now.

@jlebon
Copy link
Member

jlebon commented Mar 24, 2020

Ahh, nice catch!
/lgtm

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arithx, cgwalters, jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [arithx,cgwalters,jlebon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cgwalters
Copy link
Member Author

/hold cancel

@dustymabe
Copy link
Member

Any chance this is the source of our kola aws tests starting to "fail": coreos/fedora-coreos-pipeline#215

@dustymabe
Copy link
Member

args.build from parser.add_argument("--build", help="Build ID") doesn't seem to be used any more

@jlebon
Copy link
Member

jlebon commented Mar 25, 2020

Follow up to this here: #1283

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.

7 participants