Skip to content

Commit ef709df

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 4a43345 commit ef709df

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
@@ -169,12 +169,10 @@ func create(cmd *cobra.Command, args []string) error {
169169
}
170170
}
171171

172-
image, release, err := utils.ResolveImageName(createFlags.distro, createFlags.image, release)
173-
if err != nil {
174-
return err
175-
}
176-
177-
container, err = utils.ResolveContainerName(container, image, release)
172+
container, image, release, err := utils.ResolveContainerAndImageNames(container,
173+
createFlags.distro,
174+
createFlags.image,
175+
release)
178176
if err != nil {
179177
return err
180178
}

src/cmd/enter.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,7 @@ func enter(cmd *cobra.Command, args []string) error {
125125
}
126126
}
127127

128-
image, release, err := utils.ResolveImageName(enterFlags.distro, "", release)
129-
if err != nil {
130-
return err
131-
}
132-
133-
container, err = utils.ResolveContainerName(container, image, release)
128+
container, image, release, err := utils.ResolveContainerAndImageNames(container, enterFlags.distro, "", release)
134129
if err != nil {
135130
return err
136131
}

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
@@ -135,12 +135,7 @@ func run(cmd *cobra.Command, args []string) error {
135135

136136
command := args
137137

138-
image, release, err := utils.ResolveImageName(runFlags.distro, "", release)
139-
if err != nil {
140-
return err
141-
}
142-
143-
container, err := utils.ResolveContainerName(runFlags.container, image, release)
138+
container, image, release, err := utils.ResolveContainerAndImageNames(runFlags.container, runFlags.distro, "", release)
144139
if err != nil {
145140
return err
146141
}

src/pkg/utils/utils.go

+23-41
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)