Skip to content

Commit 28b6bac

Browse files
committed
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 fd75608. [1] Commit fd75608 containers#828 containers#838 [2] containers#977 containers#937
1 parent 99dfdf9 commit 28b6bac

File tree

5 files changed

+30
-65
lines changed

5 files changed

+30
-65
lines changed

src/cmd/create.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,10 @@ func create(cmd *cobra.Command, args []string) error {
164164
}
165165
}
166166

167-
image, release, err := utils.ResolveImageName(createFlags.distro, createFlags.image, release)
168-
if err != nil {
169-
return err
170-
}
171-
172-
container, err = utils.ResolveContainerName(container, image, release)
167+
container, image, release, err := utils.ResolveContainerAndImageNames(container,
168+
createFlags.distro,
169+
createFlags.image,
170+
release)
173171
if err != nil {
174172
return err
175173
}

src/cmd/enter.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,7 @@ func enter(cmd *cobra.Command, args []string) error {
119119
}
120120
}
121121

122-
image, release, err := utils.ResolveImageName(enterFlags.distro, "", release)
123-
if err != nil {
124-
return err
125-
}
126-
127-
container, err = utils.ResolveContainerName(container, image, release)
122+
container, image, release, err := utils.ResolveContainerAndImageNames(container, enterFlags.distro, "", release)
128123
if err != nil {
129124
return err
130125
}

src/cmd/rootMigrationPath.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,7 @@ func rootRunImpl(cmd *cobra.Command, args []string) error {
6060
return nil
6161
}
6262

63-
image, release, err := utils.ResolveImageName("", "", "")
64-
if err != nil {
65-
return err
66-
}
67-
68-
container, err := utils.ResolveContainerName("", image, release)
63+
container, image, release, err := utils.ResolveContainerAndImageNames("", "", "", "")
6964
if err != nil {
7065
return err
7166
}

src/cmd/run.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,7 @@ func run(cmd *cobra.Command, args []string) error {
130130

131131
command := args
132132

133-
image, release, err := utils.ResolveImageName(runFlags.distro, "", release)
134-
if err != nil {
135-
return err
136-
}
137-
138-
container, err := utils.ResolveContainerName(runFlags.container, image, release)
133+
container, image, release, err := utils.ResolveContainerAndImageNames(runFlags.container, runFlags.distro, "", release)
139134
if err != nil {
140135
return err
141136
}

src/pkg/utils/utils.go

Lines changed: 23 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -582,13 +582,7 @@ func SetUpConfiguration() error {
582582
}
583583
}
584584

585-
image, release, err := ResolveImageName("", "", "")
586-
if err != nil {
587-
logrus.Debugf("Setting up configuration: failed to resolve image name: %s", err)
588-
return errors.New("failed to resolve image name")
589-
}
590-
591-
container, err := ResolveContainerName("", image, release)
585+
container, _, _, err := ResolveContainerAndImageNames("", "", "", "")
592586
if err != nil {
593587
logrus.Debugf("Setting up configuration: failed to resolve container name: %s", err)
594588
return errors.New("failed to resolve container name")
@@ -693,41 +687,15 @@ func IsInsideToolboxContainer() bool {
693687
return PathExists("/run/.toolboxenv")
694688
}
695689

696-
// ResolveContainerName standardizes the name of a container
697-
//
698-
// If no container name is specified then the name of the image will be used.
699-
func ResolveContainerName(container, image, release string) (string, error) {
700-
logrus.Debug("Resolving container name")
701-
logrus.Debugf("Container: '%s'", container)
702-
logrus.Debugf("Image: '%s'", image)
703-
logrus.Debugf("Release: '%s'", release)
704-
705-
if container == "" {
706-
var err error
707-
container, err = GetContainerNamePrefixForImage(image)
708-
if err != nil {
709-
return "", err
710-
}
711-
712-
tag := ImageReferenceGetTag(image)
713-
if tag != "" {
714-
container = container + "-" + tag
715-
}
716-
}
717-
718-
logrus.Debug("Resolved container name")
719-
logrus.Debugf("Container: '%s'", container)
720-
721-
return container, nil
722-
}
723-
724-
// ResolveImageName standardizes the name of an image.
690+
// ResolveContainerAndImageNames takes care of standardizing names of containers and images.
725691
//
726692
// If no image name is specified then the base image will reflect the platform of the host (even the version).
693+
// If no container name is specified then the name of the image will be used.
727694
//
728695
// If the host system is unknown then the base image will be 'fedora-toolbox' with a default version
729-
func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, error) {
730-
logrus.Debug("Resolving image name")
696+
func ResolveContainerAndImageNames(container, distroCLI, imageCLI, releaseCLI string) (string, string, string, error) {
697+
logrus.Debug("Resolving container and image names")
698+
logrus.Debugf("Container: '%s'", container)
731699
logrus.Debugf("Distribution (CLI): '%s'", distroCLI)
732700
logrus.Debugf("Image (CLI): '%s'", imageCLI)
733701
logrus.Debugf("Release (CLI): '%s'", releaseCLI)
@@ -742,7 +710,7 @@ func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, e
742710
}
743711

744712
if distro != distroDefault && releaseCLI == "" && !viper.IsSet("general.release") {
745-
return "", "", fmt.Errorf("release not found for non-default distribution %s", distro)
713+
return "", "", "", fmt.Errorf("release not found for non-default distribution %s", distro)
746714
}
747715

748716
if releaseCLI == "" {
@@ -776,9 +744,23 @@ func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, e
776744
}
777745
}
778746

779-
logrus.Debug("Resolved image name")
747+
if container == "" {
748+
var err error
749+
container, err = GetContainerNamePrefixForImage(image)
750+
if err != nil {
751+
return "", "", "", err
752+
}
753+
754+
tag := ImageReferenceGetTag(image)
755+
if tag != "" {
756+
container = container + "-" + tag
757+
}
758+
}
759+
760+
logrus.Debug("Resolved container and image names")
761+
logrus.Debugf("Container: '%s'", container)
780762
logrus.Debugf("Image: '%s'", image)
781763
logrus.Debugf("Release: '%s'", release)
782764

783-
return image, release, nil
765+
return container, image, release, nil
784766
}

0 commit comments

Comments
 (0)