Skip to content

Commit

Permalink
cli: inconsistent precedence for registry flag
Browse files Browse the repository at this point in the history
Problem:
Commands `jaeger install`, `multicluster link` give precedence to `LINKERD_DOCKER_REGISTRY` env var, whereas commands `install`, `upgrade` and `inject` give preference to `--register` flag.

Solution:
Make the commands consitent by giving precedence to `--register` flag in all commands.

Fixes: #11115

Signed-off-by: Harsh Soni <[email protected]>
  • Loading branch information
harsh020 committed Jul 27, 2023
1 parent bc484be commit a434737
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 9 deletions.
18 changes: 13 additions & 5 deletions jaeger/cmd/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ func newCmdInstall() *cobra.Command {
var wait time.Duration
var options values.Options

// If LINKERD_DOCKER_REGISTRY is not null, use it as default registry path.
// If --registry option is provided, it will override the env variable.
defaultDockerRegistry := pkgcmd.DefaultDockerRegistry
if regOverride := os.Getenv(flags.EnvOverrideDockerRegistry); regOverride != "" {
defaultDockerRegistry = regOverride
}

cmd := &cobra.Command{
Use: "install [flags]",
Args: cobra.NoArgs,
Expand Down Expand Up @@ -75,7 +82,7 @@ A full list of configurable values can be found at https://www.github.com/linker
},
}

cmd.Flags().StringVar(&registry, "registry", pkgcmd.DefaultDockerRegistry,
cmd.Flags().StringVar(&registry, "registry", defaultDockerRegistry,
fmt.Sprintf("Docker registry to pull jaeger-webhook image from ($%s)", flags.EnvOverrideDockerRegistry))
cmd.Flags().BoolVar(&skipChecks, "skip-checks", false, `Skip checks for linkerd core control-plane existence`)
cmd.Flags().BoolVar(&ignoreCluster, "ignore-cluster", false,
Expand Down Expand Up @@ -151,13 +158,14 @@ func render(w io.Writer, valuesOverrides map[string]interface{}, registry string
}

regOrig := vals["webhook"].(map[string]interface{})["image"].(map[string]interface{})["name"].(string)

// registry variable can never be empty. The precedence are as:
// 1. --registry
// 2. EnvOverrideDockerRegistry
// 3. DefaultDockerRegistry
if registry != "" {
vals["webhook"].(map[string]interface{})["image"].(map[string]interface{})["name"] = pkgcmd.RegistryOverride(regOrig, registry)
}
// env var overrides CLI flag
if override := os.Getenv(flags.EnvOverrideDockerRegistry); override != "" {
vals["webhook"].(map[string]interface{})["image"].(map[string]interface{})["name"] = pkgcmd.RegistryOverride(regOrig, override)
}

fullValues := map[string]interface{}{
"Values": vals,
Expand Down
11 changes: 7 additions & 4 deletions multicluster/cmd/link.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ type (

func newLinkCommand() *cobra.Command {
opts, err := newLinkOptionsWithDefault()

// Override the default value with env registry path.
// If cli cmd contains --registry flag, it will override env variable.
if registry := os.Getenv(flags.EnvOverrideDockerRegistry); registry != "" {
opts.dockerRegistry = registry
}

var valuesOptions valuespkg.Options

if err != nil {
Expand Down Expand Up @@ -423,10 +430,6 @@ func buildServiceMirrorValues(opts *linkOptions) (*multicluster.Values, error) {
return nil, err
}

if reg := os.Getenv(flags.EnvOverrideDockerRegistry); reg != "" {
opts.dockerRegistry = reg
}

defaults.TargetClusterName = opts.clusterName
defaults.ServiceMirrorRetryLimit = opts.serviceMirrorRetryLimit
defaults.LogLevel = opts.logLevel
Expand Down

0 comments on commit a434737

Please sign in to comment.