Skip to content

Commit 235e2d2

Browse files
debarshiraydebarshiray
committed
Revert "cmd, pkg/utils: Split distro and release parsing and ..."
The idea of splitting ResolveContainerAndImageNames into two public functions [1] didn't turn out to be so useful [2]. Splitting things even further might make it worse. A better approach might be to (re-)unify the code further. This is the first step towards that. This reverts the following commits: * 5c8ad7a * 02f45fd * 8b6418d ... but retains the test cases that were not tied to the changes in those commits. [1] Commit fd75608 containers#828 containers#838 [2] containers#977 containers#937
1 parent 4f78c5e commit 235e2d2

File tree

9 files changed

+34
-282
lines changed

9 files changed

+34
-282
lines changed

‎src/cmd/create.go

+5-11
Original file line numberDiff line numberDiff line change
@@ -154,23 +154,17 @@ func create(cmd *cobra.Command, args []string) error {
154154
}
155155
}
156156

157-
distro, err := utils.ResolveDistro(createFlags.distro)
158-
if err != nil {
159-
err := createErrorInvalidDistro()
160-
return err
161-
}
162-
163-
release := createFlags.release
164-
if release != "" {
157+
var release string
158+
if createFlags.release != "" {
165159
var err error
166-
release, err = utils.ParseRelease(distro, release)
160+
release, err = utils.ParseRelease(createFlags.distro, createFlags.release)
167161
if err != nil {
168-
err := createErrorInvalidRelease(distro)
162+
err := createErrorInvalidRelease()
169163
return err
170164
}
171165
}
172166

173-
image, release, err := utils.ResolveImageName(distro, createFlags.image, release)
167+
image, release, err := utils.ResolveImageName(createFlags.distro, createFlags.image, release)
174168
if err != nil {
175169
return err
176170
}

‎src/cmd/enter.go

+5-11
Original file line numberDiff line numberDiff line change
@@ -107,25 +107,19 @@ func enter(cmd *cobra.Command, args []string) error {
107107
}
108108
}
109109

110-
distro, err := utils.ResolveDistro(enterFlags.distro)
111-
if err != nil {
112-
err := createErrorInvalidDistro()
113-
return err
114-
}
115-
116-
release := enterFlags.release
117-
if release != "" {
110+
var release string
111+
if enterFlags.release != "" {
118112
defaultContainer = false
119113

120114
var err error
121-
release, err = utils.ParseRelease(distro, release)
115+
release, err = utils.ParseRelease(enterFlags.distro, enterFlags.release)
122116
if err != nil {
123-
err := createErrorInvalidRelease(distro)
117+
err := createErrorInvalidRelease()
124118
return err
125119
}
126120
}
127121

128-
image, release, err := utils.ResolveImageName(distro, "", release)
122+
image, release, err := utils.ResolveImageName(enterFlags.distro, "", release)
129123
if err != nil {
130124
return err
131125
}

‎src/cmd/run.go

+5-11
Original file line numberDiff line numberDiff line change
@@ -107,20 +107,14 @@ func run(cmd *cobra.Command, args []string) error {
107107
}
108108
}
109109

110-
distro, err := utils.ResolveDistro(runFlags.distro)
111-
if err != nil {
112-
err := createErrorInvalidDistro()
113-
return err
114-
}
115-
116-
release := runFlags.release
117-
if release != "" {
110+
var release string
111+
if runFlags.release != "" {
118112
defaultContainer = false
119113

120114
var err error
121-
release, err = utils.ParseRelease(distro, release)
115+
release, err = utils.ParseRelease(runFlags.distro, runFlags.release)
122116
if err != nil {
123-
err := createErrorInvalidRelease(distro)
117+
err := createErrorInvalidRelease()
124118
return err
125119
}
126120
}
@@ -136,7 +130,7 @@ func run(cmd *cobra.Command, args []string) error {
136130

137131
command := args
138132

139-
image, release, err := utils.ResolveImageName(distro, "", release)
133+
image, release, err := utils.ResolveImageName(runFlags.distro, "", release)
140134
if err != nil {
141135
return err
142136
}

‎src/cmd/utils.go

+1-12
Original file line numberDiff line numberDiff line change
@@ -81,20 +81,9 @@ func createErrorInvalidContainer(containerArg string) error {
8181
return errors.New(errMsg)
8282
}
8383

84-
func createErrorInvalidDistro() error {
85-
var builder strings.Builder
86-
fmt.Fprintf(&builder, "invalid argument for '--distro'\n")
87-
fmt.Fprintf(&builder, "Supported values are: %s\n", strings.Join(utils.GetSupportedDistros(), " "))
88-
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)
89-
90-
errMsg := builder.String()
91-
return errors.New(errMsg)
92-
}
93-
94-
func createErrorInvalidRelease(distro string) error {
84+
func createErrorInvalidRelease() error {
9585
var builder strings.Builder
9686
fmt.Fprintf(&builder, "invalid argument for '--release'\n")
97-
fmt.Fprintf(&builder, "Supported values for distribution %s are in format: %s\n", distro, utils.GetReleaseFormat(distro))
9887
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)
9988

10089
errMsg := builder.String()

‎src/pkg/utils/utils.go

+12-67
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ type Distro struct {
4444
ContainerNamePrefix string
4545
ImageBasename string
4646
ParseRelease ParseReleaseFunc
47-
ReleaseFormat string
4847
Registry string
4948
Repository string
5049
RepositoryNeedsRelease bool
@@ -61,10 +60,6 @@ const (
6160
ContainerNameRegexp = "[a-zA-Z0-9][a-zA-Z0-9_.-]*"
6261
)
6362

64-
var (
65-
ErrUnsupportedDistro = errors.New("linux distribution is not supported")
66-
)
67-
6863
var (
6964
containerNamePrefixDefault = "fedora-toolbox"
7065

@@ -103,7 +98,6 @@ var (
10398
"fedora-toolbox",
10499
"fedora-toolbox",
105100
parseReleaseFedora,
106-
"<release>/f<release>",
107101
"registry.fedoraproject.org",
108102
"",
109103
false,
@@ -112,7 +106,6 @@ var (
112106
"rhel-toolbox",
113107
"toolbox",
114108
parseReleaseRHEL,
115-
"<major.minor>",
116109
"registry.access.redhat.com",
117110
"ubi8",
118111
false,
@@ -253,8 +246,8 @@ func getDefaultImageForDistro(distro, release string) string {
253246
panic("distro not specified")
254247
}
255248

256-
if !IsDistroSupported(distro) {
257-
distro = distroDefault
249+
if _, supportedDistro := supportedDistros[distro]; !supportedDistro {
250+
distro = "fedora"
258251
}
259252

260253
distroObj, supportedDistro := supportedDistros[distro]
@@ -418,20 +411,6 @@ func GetMountOptions(target string) (string, error) {
418411
return mountOptions, nil
419412
}
420413

421-
// GetReleaseFormat returns the format string signifying supported release
422-
// version formats.
423-
//
424-
// distro should be value found under ID in os-release.
425-
//
426-
// If distro is unsupported an empty string is returned.
427-
func GetReleaseFormat(distro string) string {
428-
if !IsDistroSupported(distro) {
429-
return ""
430-
}
431-
432-
return supportedDistros[distro].ReleaseFormat
433-
}
434-
435414
func GetRuntimeDirectory(targetUser *user.User) (string, error) {
436415
gid, err := strconv.Atoi(targetUser.Gid)
437416
if err != nil {
@@ -488,14 +467,6 @@ func HumanDuration(duration int64) string {
488467
return units.HumanDuration(time.Since(time.Unix(duration, 0))) + " ago"
489468
}
490469

491-
// IsDistroSupported signifies if a distribution has a toolbx image for it.
492-
//
493-
// distro should be value found under ID in os-release.
494-
func IsDistroSupported(distro string) bool {
495-
_, ok := supportedDistros[distro]
496-
return ok
497-
}
498-
499470
// ImageReferenceCanBeID checks if 'image' might be the ID of an image
500471
func ImageReferenceCanBeID(image string) bool {
501472
matched, err := regexp.MatchString("^[a-f0-9]{6,64}$", image)
@@ -640,18 +611,16 @@ func ShortID(id string) string {
640611
return id
641612
}
642613

643-
// ParseRelease assesses if the requested version of a distribution is in
644-
// the correct format.
645-
//
646-
// If distro is an empty string, a default value (value under the
647-
// 'general.distro' key in a config file or 'fedora') is assumed.
648614
func ParseRelease(distro, release string) (string, error) {
649615
if distro == "" {
650-
distro, _ = ResolveDistro(distro)
616+
distro = distroDefault
617+
if viper.IsSet("general.distro") {
618+
distro = viper.GetString("general.distro")
619+
}
651620
}
652621

653-
if !IsDistroSupported(distro) {
654-
distro = distroDefault
622+
if _, supportedDistro := supportedDistros[distro]; !supportedDistro {
623+
distro = "fedora"
655624
}
656625

657626
distroObj, supportedDistro := supportedDistros[distro]
@@ -756,33 +725,6 @@ func ResolveContainerName(container, image, release string) (string, error) {
756725
return container, nil
757726
}
758727

759-
// ResolveDistro assess if the requested distribution is supported and provides
760-
// a default value if none is requested.
761-
//
762-
// If distro is empty, and the "general.distro" key in a config file is set,
763-
// the value is read from the config file. If the key is not set, the default
764-
// value ('fedora') is used instead.
765-
func ResolveDistro(distro string) (string, error) {
766-
logrus.Debug("Resolving distribution")
767-
logrus.Debugf("Distribution: %s", distro)
768-
769-
if distro == "" {
770-
distro = distroDefault
771-
if viper.IsSet("general.distro") {
772-
distro = viper.GetString("general.distro")
773-
}
774-
}
775-
776-
if !IsDistroSupported(distro) {
777-
return "", ErrUnsupportedDistro
778-
}
779-
780-
logrus.Debug("Resolved distribution")
781-
logrus.Debugf("Distribution: %s", distro)
782-
783-
return distro, nil
784-
}
785-
786728
// ResolveImageName standardizes the name of an image.
787729
//
788730
// If no image name is specified then the base image will reflect the platform of the host (even the version).
@@ -797,7 +739,10 @@ func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, e
797739
distro, image, release := distroCLI, imageCLI, releaseCLI
798740

799741
if distroCLI == "" {
800-
distro, _ = ResolveDistro(distroCLI)
742+
distro = distroDefault
743+
if viper.IsSet("general.distro") {
744+
distro = viper.GetString("general.distro")
745+
}
801746
}
802747

803748
if distro != distroDefault && releaseCLI == "" && !viper.IsSet("general.release") {

0 commit comments

Comments
 (0)