pkg/utils: Re-unify container & image name resolution

The idea of splitting ResolveContainerAndImageNames into two public
functions [1] didn't turn out to be so useful [2].  It pushed the
burden on the callers, who needed to carefully call the split
functions in the right order, because the container, distro, image and
release values are very tightly related.  This opens the door for
mistakes.

A better approach would be to restore ResolveContainerAndImageNames as
the single public API.  If necessary it could be internally split into
smaller private functions.  It would keep things simple for the
callers.

Note that this commit doesn't include the private split.  If necessary,
it can be done in future.

This reverts commit fd756089ef.

[1] Commit fd756089ef
    https://github.com/containers/toolbox/pull/828
    https://github.com/containers/toolbox/pull/838

[2] https://github.com/containers/toolbox/pull/977

https://github.com/containers/toolbox/issues/937
https://github.com/containers/toolbox/pull/1080
This commit is contained in:
Debarshi Ray 2022-07-29 20:39:31 +02:00
parent 0e66af91fe
commit 344dda6d8d
5 changed files with 30 additions and 65 deletions

View File

@ -164,12 +164,10 @@ func create(cmd *cobra.Command, args []string) error {
}
}
image, release, err := utils.ResolveImageName(createFlags.distro, createFlags.image, release)
if err != nil {
return err
}
container, err = utils.ResolveContainerName(container, image, release)
container, image, release, err := utils.ResolveContainerAndImageNames(container,
createFlags.distro,
createFlags.image,
release)
if err != nil {
return err
}

View File

@ -119,12 +119,7 @@ func enter(cmd *cobra.Command, args []string) error {
}
}
image, release, err := utils.ResolveImageName(enterFlags.distro, "", release)
if err != nil {
return err
}
container, err = utils.ResolveContainerName(container, image, release)
container, image, release, err := utils.ResolveContainerAndImageNames(container, enterFlags.distro, "", release)
if err != nil {
return err
}

View File

@ -60,12 +60,7 @@ func rootRunImpl(cmd *cobra.Command, args []string) error {
return nil
}
image, release, err := utils.ResolveImageName("", "", "")
if err != nil {
return err
}
container, err := utils.ResolveContainerName("", image, release)
container, image, release, err := utils.ResolveContainerAndImageNames("", "", "", "")
if err != nil {
return err
}

View File

@ -130,12 +130,7 @@ func run(cmd *cobra.Command, args []string) error {
command := args
image, release, err := utils.ResolveImageName(runFlags.distro, "", release)
if err != nil {
return err
}
container, err := utils.ResolveContainerName(runFlags.container, image, release)
container, image, release, err := utils.ResolveContainerAndImageNames(runFlags.container, runFlags.distro, "", release)
if err != nil {
return err
}

View File

@ -586,13 +586,7 @@ func SetUpConfiguration() error {
}
}
image, release, err := ResolveImageName("", "", "")
if err != nil {
logrus.Debugf("Setting up configuration: failed to resolve image name: %s", err)
return errors.New("failed to resolve image name")
}
container, err := ResolveContainerName("", image, release)
container, _, _, err := ResolveContainerAndImageNames("", "", "", "")
if err != nil {
logrus.Debugf("Setting up configuration: failed to resolve container name: %s", err)
return errors.New("failed to resolve container name")
@ -697,41 +691,15 @@ func IsInsideToolboxContainer() bool {
return PathExists("/run/.toolboxenv")
}
// ResolveContainerName standardizes the name of a container
//
// If no container name is specified then the name of the image will be used.
func ResolveContainerName(container, image, release string) (string, error) {
logrus.Debug("Resolving container name")
logrus.Debugf("Container: '%s'", container)
logrus.Debugf("Image: '%s'", image)
logrus.Debugf("Release: '%s'", release)
if container == "" {
var err error
container, err = getContainerNamePrefixForImage(image)
if err != nil {
return "", err
}
tag := ImageReferenceGetTag(image)
if tag != "" {
container = container + "-" + tag
}
}
logrus.Debug("Resolved container name")
logrus.Debugf("Container: '%s'", container)
return container, nil
}
// ResolveImageName standardizes the name of an image.
// ResolveContainerAndImageNames takes care of standardizing names of containers and images.
//
// If no image name is specified then the base image will reflect the platform of the host (even the version).
// If no container name is specified then the name of the image will be used.
//
// If the host system is unknown then the base image will be 'fedora-toolbox' with a default version
func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, error) {
logrus.Debug("Resolving image name")
func ResolveContainerAndImageNames(container, distroCLI, imageCLI, releaseCLI string) (string, string, string, error) {
logrus.Debug("Resolving container and image names")
logrus.Debugf("Container: '%s'", container)
logrus.Debugf("Distribution (CLI): '%s'", distroCLI)
logrus.Debugf("Image (CLI): '%s'", imageCLI)
logrus.Debugf("Release (CLI): '%s'", releaseCLI)
@ -746,7 +714,7 @@ func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, e
}
if distro != distroDefault && releaseCLI == "" && !viper.IsSet("general.release") {
return "", "", fmt.Errorf("release not found for non-default distribution %s", distro)
return "", "", "", fmt.Errorf("release not found for non-default distribution %s", distro)
}
if releaseCLI == "" {
@ -780,9 +748,23 @@ func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, e
}
}
logrus.Debug("Resolved image name")
if container == "" {
var err error
container, err = getContainerNamePrefixForImage(image)
if err != nil {
return "", "", "", err
}
tag := ImageReferenceGetTag(image)
if tag != "" {
container = container + "-" + tag
}
}
logrus.Debug("Resolved container and image names")
logrus.Debugf("Container: '%s'", container)
logrus.Debugf("Image: '%s'", image)
logrus.Debugf("Release: '%s'", release)
return image, release, nil
return container, image, release, nil
}