Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

system test image: bump to 20220615 #14529

Merged

Conversation

edsantiago
Copy link
Member

@edsantiago edsantiago commented Jun 8, 2022

Changes:

  • use --timestamp option to produce 'created' stamps
    that can be reliably tested in the image-history test

  • podman now supports manifest & multiarch run, so we
    no longer need buildah

  • bump up base alpine & busybox images

This turned out to be WAY more complicated than it should've been,
because:

  • alpine 3.14 fixed 'date -Iseconds' to include a colon in
    the TZ offset ("-07:00", was "-0700"). This is now consistent
    with GNU date's --iso-8601 format, yay, so we can eliminate
    a minor workaround.

  • with --timestamp, all ADDed files are set to that timestamp,
    including the custom-reference-timestamp file that many tests
    rely on. So we need to split the build into two steps. But:

  • ...with a two-step build I need to use --squash-all, not --squash, but:

  • ... (deep sigh) --squash-all doesn't work with --timestamp (build with --squash-all + --timestamp clobbers timestamps in preexisting layers #14536)
    so we need to alter existing tests to deal with new image layers.

  • And, long and sordid story relating to --rootfs. TL;DR that option
    only worked by a miracle relating to something special in one
    specific test image; it doesn't work with any other images. Fix
    seems to be complicated, so we're bypassing with a FIXME (Unhealthy pod(created via podman play kube) doesn't restart #14505).

And, unrelated:

  • remove obsolete skip and workaround in run-basic test (dating
    back to varlink days)
  • add a pause-image cleanup to avoid icky red warnings in logs

Fixes: #14456

Signed-off-by: Ed Santiago [email protected]

None

@openshift-ci openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 8, 2022
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM but it looks like the podman run --tz test needs to be updated as well.

@edsantiago edsantiago force-pushed the testimage_with_consistent_timestamps branch 2 times, most recently from e9fc656 to 94ebe3e Compare June 8, 2022 14:23
@edsantiago edsantiago changed the title system test image: bump to 20220606 system test image: bump to 20220608 Jun 8, 2022
@rhatdan
Copy link
Member

rhatdan commented Jun 8, 2022

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 8, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 8, 2022
@edsantiago
Copy link
Member Author

LGTM but it looks like the podman run --tz test needs to be updated as well.

This (like everything else in this PR) was much more complicated than I had expected. TL;DR my two-stage build solved this, but that broke the image tree test. I fixed image tree by adding --squash-all, but didn't rerun the entire test suite and didn't notice that the TZ test had broken (because --squash-all + --timestamp apparently clobbers timestamps that it shouldn't; I will look into this later). I've rebuilt the testimage with one --squash-all and one --squash, and TZ and tree tests now pass on my system, and we will now see what other surprises lurk.

That may have been the longest TL;DR I've ever written. Sorry.

@edsantiago
Copy link
Member Author

Okay, looks like I really need to look into this squash-all situation.

@edsantiago
Copy link
Member Author

Filed #14536

Changes:
 - use --timestamp option to produce 'created' stamps
   that can be reliably tested in the image-history test

 - podman now supports manifest & multiarch run, so we
   no longer need buildah

 - bump up base alpine & busybox images

This turned out to be WAY more complicated than it should've been,
because:

 - alpine 3.14 fixed 'date -Iseconds' to include a colon in
   the TZ offset ("-07:00", was "-0700"). This is now consistent
   with GNU date's --iso-8601 format, yay, so we can eliminate
   a minor workaround.

 - with --timestamp, all ADDed files are set to that timestamp,
   including the custom-reference-timestamp file that many tests
   rely on. So we need to split the build into two steps. But:

 - ...with a two-step build I need to use --squash-all, not --squash, but:

 - ... (deep sigh) --squash-all doesn't work with --timestamp (containers#14536)
   so we need to alter existing tests to deal with new image layers.

 - And, long and sordid story relating to --rootfs. TL;DR that option
   only worked by a miracle relating to something special in one
   specific test image; it doesn't work with any other images. Fix
   seems to be complicated, so we're bypassing with a FIXME (containers#14505).

And, unrelated:

 - remove obsolete skip and workaround in run-basic test (dating
   back to varlink days)
 - add a pause-image cleanup to avoid icky red warnings in logs

Fixes: containers#14456

Signed-off-by: Ed Santiago <[email protected]>
@edsantiago edsantiago force-pushed the testimage_with_consistent_timestamps branch from 94ebe3e to 0a202a9 Compare June 15, 2022 19:31
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 15, 2022
@edsantiago edsantiago changed the title system test image: bump to 20220608 system test image: bump to 20220615 Jun 15, 2022
@edsantiago edsantiago removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 15, 2022
@edsantiago
Copy link
Member Author

@containers/podman-maintainers this is ready. As a reminder, it was supposed to be a five-minute fix for test failures seen by the multiarch team.

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 16, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, vrothberg

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [edsantiago,vrothberg]

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

@vrothberg
Copy link
Member

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 16, 2022
@openshift-ci openshift-ci bot merged commit 78c149f into containers:main Jun 16, 2022
@edsantiago edsantiago deleted the testimage_with_consistent_timestamps branch June 16, 2022 21:44
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timestamps in 'image list' inconsistent with 'image history'
4 participants