Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SKAFFOLD_UPDATE_CHECK should also be a global flag #4510

Merged
merged 1 commit into from
Jul 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/skaffold/app/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
rootCmd.PersistentFlags().IntVar(&defaultColor, "color", int(color.DefaultColorCode), "Specify the default output color in ANSI escape codes")
rootCmd.PersistentFlags().BoolVar(&forceColors, "force-colors", false, "Always print color codes (hidden)")
rootCmd.PersistentFlags().BoolVar(&interactive, "interactive", true, "Allow user prompts for more information")
rootCmd.PersistentFlags().BoolVar(&update.EnableCheck, "update-check", true, "Check for a more recent version of Skaffold")
rootCmd.PersistentFlags().MarkHidden("force-colors")

setFlagsFromEnvVariables(rootCmd)
Expand Down
2 changes: 2 additions & 0 deletions docs/content/en/docs/references/cli/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Env vars:

* `SKAFFOLD_COLOR` (same as `--color`)
* `SKAFFOLD_INTERACTIVE` (same as `--interactive`)
* `SKAFFOLD_UPDATE_CHECK` (same as `--update-check`)
* `SKAFFOLD_VERBOSITY` (same as `--verbosity`)

### skaffold build
Expand Down Expand Up @@ -697,6 +698,7 @@ The following options can be passed to any command:

--color=34: Specify the default output color in ANSI escape codes
--interactive=true: Allow user prompts for more information
--update-check=true: Check for a more recent version of Skaffold
-v, --verbosity='warning': Log level (debug, info, warn, error, fatal, panic)


Expand Down
2 changes: 0 additions & 2 deletions pkg/skaffold/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ const (
// DefaultDebugHelpersRegistry is the default location used for the helper images for `debug`.
DefaultDebugHelpersRegistry = "gcr.io/gcp-dev-tools/duct-tape"

UpdateCheckEnvironmentVariable = "SKAFFOLD_UPDATE_CHECK"

DefaultSkaffoldDir = ".skaffold"
DefaultCacheFile = "cache"

Expand Down
15 changes: 4 additions & 11 deletions pkg/skaffold/update/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ import (
"fmt"
"io/ioutil"
"net/http"
"os"
"strings"

"github.com/blang/semver"
"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
)

// EnableCheck enabled the check for a more recent version of Skaffold.
var EnableCheck bool

// For testing
var (
GetLatestAndCurrentVersion = getLatestAndCurrentVersion
isConfigUpdateCheckEnabled = config.IsUpdateCheckEnabled
getEnv = os.Getenv
)

const LatestVersionURL = "https://storage.googleapis.com/skaffold/releases/latest/VERSION"
Expand All @@ -48,14 +48,7 @@ func IsUpdateCheckEnabled(configfile string) bool {
return false
}

return isUpdateCheckEnabledByEnvOrConfig(configfile)
}

func isUpdateCheckEnabledByEnvOrConfig(configfile string) bool {
if v := getEnv(constants.UpdateCheckEnvironmentVariable); v != "" {
return strings.ToLower(v) == "true"
}
return isConfigUpdateCheckEnabled(configfile)
return EnableCheck && isConfigUpdateCheckEnabled(configfile)
}

// getLatestAndCurrentVersion uses a VERSION file stored on GCS to determine the latest released version
Expand Down
38 changes: 21 additions & 17 deletions pkg/skaffold/update/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,42 +22,46 @@ import (
"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestIsUpdateCheckEnabledByEnvOrConfig(t *testing.T) {
func TestIsUpdateCheckEnabled(t *testing.T) {
tests := []struct {
description string
envVariable string
enabled bool
configCheck bool
expected bool
}{
{
description: "env variable is set to true",
envVariable: "true",
expected: true,
},
{
description: "env variable is set to false",
envVariable: "false",
description: "globally disabled - disabled in config -> disabled",
enabled: false,
configCheck: false,
expected: false,
},
{
description: "env variable is set to random string",
envVariable: "foo",
description: "globally enabled - disabled in config -> disabled",
enabled: true,
configCheck: false,
expected: false,
},
{
description: "env variable is empty and config is enabled",
description: "globally disabled - enabled in config -> disabled",
enabled: false,
configCheck: true,
expected: true,
expected: false,
},
{
description: "env variable is false but Global update-check config is true",
envVariable: "false",
description: "globally enabled - enabled in config -> enabled",
enabled: true,
configCheck: true,
expected: true,
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&EnableCheck, test.enabled)
t.Override(&isConfigUpdateCheckEnabled, func(string) bool { return test.configCheck })
t.Override(&getEnv, func(string) string { return test.envVariable })
t.CheckDeepEqual(test.expected, isUpdateCheckEnabledByEnvOrConfig("dummyconfig"))

isEnabled := IsUpdateCheckEnabled("dummyconfig")

t.CheckDeepEqual(test.expected, isEnabled)
})
}
}