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

libimage: Untag should error for non existent name #1701

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Oct 20, 2023

podman untag should error out of a name is given which does not exists for the given image. This regression was added in commit 465dd3e.

There was even a test which meant to check for it but unfortunately it did not actually check for what it should. The doNotExist check failed early to the upper case in the repo name.
The tests have been updated to check for actual error messages to show ensure it is failing for the right reason.
This also showed that normalizing name message was included twice so I removed one case to not stutter.

Fixes 465dd3e ("libimage: support parallel tag/untag")

podman untag should error out of a name is given which does not exists
for the given image. This regression was added in commit 465dd3e.

There was even a test which meant to check for it but unfortunately it
did not actually check for what it should. The doNotExist check failed
early to the upper case in the repo name.
The tests have been updated to check for actual error messages to show
ensure it is failing for the right reason.
This also showed that `normalizing name` message was included twice so
I removed one case to not stutter.

Fixes 465dd3e ("libimage: support parallel tag/untag")

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
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

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, 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:

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

@@ -587,6 +587,9 @@ func (i *Image) Tag(name string) error {
return i.reload()
}

// to have some symmetry with the errors from containers/storage.
var errTagUnknown = errors.New("tag not known")
Copy link
Member

@rhatdan rhatdan Oct 20, 2023

Choose a reason for hiding this comment

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

Should this be exported in libimage/define?

Copy link
Member

Choose a reason for hiding this comment

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

I don´t think we need to. I cannot think of a scenario where users shouldn't error out.

Copy link
Member Author

Choose a reason for hiding this comment

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

this is c/common so no and it just brings back the error removed in 465dd3e.

I don't see the need to export it unless someone wants to match on this specific error but for now this is not the case and we can always export it later if needed

@rhatdan
Copy link
Member

rhatdan commented Oct 20, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Oct 20, 2023
@openshift-ci openshift-ci bot merged commit 3db9354 into containers:main Oct 20, 2023
7 checks passed
@Luap99 Luap99 deleted the untag-err branch October 20, 2023 12:31
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