Skip to content

imagebuildah: set len(short_image_id) to 12#4660

Merged
rhatdan merged 1 commit intocontainers:mainfrom
danishprakash:img-id-len
Mar 18, 2023
Merged

imagebuildah: set len(short_image_id) to 12#4660
rhatdan merged 1 commit intocontainers:mainfrom
danishprakash:img-id-len

Conversation

@danishprakash
Copy link
Copy Markdown
Member

@danishprakash danishprakash commented Mar 15, 2023

What type of PR is this?

/kind bug

What this PR does / why we need it:

Buildah bud prints a 11-digit hash during build unlike docker, which dumps a 12-digit hash. Further looking at the relevant commit, this looks like a typo since the commit clearly intends to dump the 12-digit hash aiming for parity with docker.

$ ./bin/buildah bud -t test -f /tmp/tmp.6WrKR1Ehxm/Dockerfile .
STEP 1/1: FROM alpine
COMMIT test
--> b2aa39c304c2  <<===
Successfully tagged localhost/test:latest
Successfully tagged docker.io/library/alpine:latest
b2aa39c304c27b96c1fef0c06bee651ac9241d49c4fe34381cab8453f9a89c7d

Refers containers/podman#5012 & #2124

Does this PR introduce a user-facing change?

buildah bud prints a 12-digit image identifier instead of an 11-digit one

Signed-off-by: danishprakash danish.prakash@suse.com

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 15, 2023
@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Mar 15, 2023

Can you add
[NO NEW TESTS NEEDED]
to your description or add a test, and then repush.

LGTM

@flouthoc @nalind @umohnani8 @vrothberg @giuseppe PTAL

Copy link
Copy Markdown
Member

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

LGTM once the tests are hip

Thanks @danishprakash !

@flouthoc
Copy link
Copy Markdown
Collaborator

LGTM but a small test will ensure we don't regress this accidentally again in future.

@danishprakash danishprakash force-pushed the img-id-len branch 2 times, most recently from fd680ca to 8c501e6 Compare March 16, 2023 10:06
* tests/bud.bats: add test

Signed-off-by: danishprakash <danish.prakash@suse.com>
@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Mar 18, 2023

/approve
/lgtm

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Mar 18, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danishprakash, rhatdan

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:

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

@rhatdan rhatdan merged commit 61ba326 into containers:main Mar 18, 2023
flouthoc added a commit to flouthoc/podman that referenced this pull request Apr 10, 2023
After containers/buildah#4660 buildah
spits a 12 letter image short id instead of 13 so lets honor that.

Signed-off-by: Aditya R <arajan@redhat.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved kind/bug Categorizes issue or PR as related to a bug. lgtm locked - please file new issue/PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants