Skip to content

Commit

Permalink
pkg/utils: Re-unify container & image name resolution
Browse files Browse the repository at this point in the history
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 fd75608.

[1] Commit fd75608
    containers#828
    containers#838

[2] containers#977

containers#937
  • Loading branch information
debarshiray committed Sep 1, 2022
1 parent 235e2d2 commit 0fca652
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 65 deletions.
10 changes: 4 additions & 6 deletions src/cmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 1 addition & 6 deletions src/cmd/enter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 1 addition & 6 deletions src/cmd/rootMigrationPath.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 1 addition & 6 deletions src/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
64 changes: 23 additions & 41 deletions src/pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -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 == "" {
Expand Down Expand Up @@ -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
}

0 comments on commit 0fca652

Please sign in to comment.