Skip to content

Commit 344dda6

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 containers#1080
1 parent 0e66af9 commit 344dda6

File tree

5 files changed

+30
-65
lines changed

5 files changed

+30
-65
lines changed

src/cmd/create.go

+4-6
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

+1-6
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

+1-6
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

+1-6
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

+23-41
Original file line numberDiff line numberDiff line change
@@ -586,13 +586,7 @@ func SetUpConfiguration() error {
586586
}
587587
}
588588

589-
image, release, err := ResolveImageName("", "", "")
590-
if err != nil {
591-
logrus.Debugf("Setting up configuration: failed to resolve image name: %s", err)
592-
return errors.New("failed to resolve image name")
593-
}
594-
595-
container, err := ResolveContainerName("", image, release)
589+
container, _, _, err := ResolveContainerAndImageNames("", "", "", "")
596590
if err != nil {
597591
logrus.Debugf("Setting up configuration: failed to resolve container name: %s", err)
598592
return errors.New("failed to resolve container name")
@@ -697,41 +691,15 @@ func IsInsideToolboxContainer() bool {
697691
return PathExists("/run/.toolboxenv")
698692
}
699693

700-
// ResolveContainerName standardizes the name of a container
701-
//
702-
// If no container name is specified then the name of the image will be used.
703-
func ResolveContainerName(container, image, release string) (string, error) {
704-
logrus.Debug("Resolving container name")
705-
logrus.Debugf("Container: '%s'", container)
706-
logrus.Debugf("Image: '%s'", image)
707-
logrus.Debugf("Release: '%s'", release)
708-
709-
if container == "" {
710-
var err error
711-
container, err = getContainerNamePrefixForImage(image)
712-
if err != nil {
713-
return "", err
714-
}
715-
716-
tag := ImageReferenceGetTag(image)
717-
if tag != "" {
718-
container = container + "-" + tag
719-
}
720-
}
721-
722-
logrus.Debug("Resolved container name")
723-
logrus.Debugf("Container: '%s'", container)
724-
725-
return container, nil
726-
}
727-
728-
// ResolveImageName standardizes the name of an image.
694+
// ResolveContainerAndImageNames takes care of standardizing names of containers and images.
729695
//
730696
// If no image name is specified then the base image will reflect the platform of the host (even the version).
697+
// If no container name is specified then the name of the image will be used.
731698
//
732699
// If the host system is unknown then the base image will be 'fedora-toolbox' with a default version
733-
func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, error) {
734-
logrus.Debug("Resolving image name")
700+
func ResolveContainerAndImageNames(container, distroCLI, imageCLI, releaseCLI string) (string, string, string, error) {
701+
logrus.Debug("Resolving container and image names")
702+
logrus.Debugf("Container: '%s'", container)
735703
logrus.Debugf("Distribution (CLI): '%s'", distroCLI)
736704
logrus.Debugf("Image (CLI): '%s'", imageCLI)
737705
logrus.Debugf("Release (CLI): '%s'", releaseCLI)
@@ -746,7 +714,7 @@ func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, e
746714
}
747715

748716
if distro != distroDefault && releaseCLI == "" && !viper.IsSet("general.release") {
749-
return "", "", fmt.Errorf("release not found for non-default distribution %s", distro)
717+
return "", "", "", fmt.Errorf("release not found for non-default distribution %s", distro)
750718
}
751719

752720
if releaseCLI == "" {
@@ -780,9 +748,23 @@ func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, e
780748
}
781749
}
782750

783-
logrus.Debug("Resolved image name")
751+
if container == "" {
752+
var err error
753+
container, err = getContainerNamePrefixForImage(image)
754+
if err != nil {
755+
return "", "", "", err
756+
}
757+
758+
tag := ImageReferenceGetTag(image)
759+
if tag != "" {
760+
container = container + "-" + tag
761+
}
762+
}
763+
764+
logrus.Debug("Resolved container and image names")
765+
logrus.Debugf("Container: '%s'", container)
784766
logrus.Debugf("Image: '%s'", image)
785767
logrus.Debugf("Release: '%s'", release)
786768

787-
return image, release, nil
769+
return container, image, release, nil
788770
}

0 commit comments

Comments
 (0)