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

Show update check message to users once in 24 hours #5795

Merged

Conversation

tejal29
Copy link
Contributor

@tejal29 tejal29 commented May 6, 2021

Relates to #5711

Update check prompts are shown on every command run.
This could annoy the users and get them to switch off update checks.

In this PR

  1. Add a config section update with last-prompted field.
  2. Add logic to display update check prompt only once in 24 hours.

This does not change the existing update check behavior. See #5741 for that.

Testing done:

  1. Build a binary with latest version & make sure
    a) update check message is not shown.
    b) update-config is not updated with last-prompted
GOOS=darwin GOARCH=amd64 CGO_ENABLED=1 \
go build -gcflags="all=-N -l" -tags "release " -ldflags "-X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.version=v1.24.0 -s -w " -o out/skaffold github.com/GoogleContainerTools/skaffold/cmd/skaffold
.➜  multi-config-microservices git:(show_update_check_dailu) ✗ ../../out/skaffold build -d gcr.io/tejal-gke1
Generating tags...
 - base -> gcr.io/tejal-gke1/base:v1.23.0-36-gbf79f0788
 - leeroy-app -> gcr.io/tejal-gke1/leeroy-app:v1.23.0-36-gbf79f0788
 - leeroy-web -> gcr.io/tejal-gke1/leeroy-web:v1.23.0-36-gbf79f0788
Checking cache...
 - base: Found Remotely
 - leeroy-app: Found Remotely
 - leeroy-web: Found Remotely                  

➜  multi-config-microservices git:(show_update_check_dailu) ✗ cat ~/.skaffold/config
global:
  survey:
    last-prompted: "2021-04-27T13:32:59-07:00"
  collect-metrics: true
kubeContexts: []
  1. Build a binary with older version & make sure
    a) update message is shown
➜ GOOS=darwin GOARCH=amd64 CGO_ENABLED=1 \
 go build -gcflags="all=-N -l" -tags "release " -ldflags "-X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.version=v1.22.0 -s -w " -o out/skaffold github.com/GoogleContainerTools/skaffold/cmd/skaffold

➜ ../../out/skaffold build -d gcr.io/tejal-gke1
Generating tags...
 - base -> gcr.io/tejal-gke1/base:v1.23.0-36-gbf79f0788
 - leeroy-app -> gcr.io/tejal-gke1/leeroy-app:v1.23.0-36-gbf79f0788
 - leeroy-web -> gcr.io/tejal-gke1/leeroy-web:v1.23.0-36-gbf79f0788
Checking cache...
 - base: Found Remotely
 - leeroy-app: Found Remotely
 - leeroy-web: Found Remotely
There is a new version (1.23.0) of Skaffold available. Download it from:
  https://github.com/GoogleContainerTools/skaffold/releases/tag/v1.23.0

b) update-config is updated with last-prompted

➜ cat ~/.skaffold/config                       
global:
  survey:
    last-prompted: "2021-04-27T13:32:59-07:00"
  collect-metrics: true
  update-config:
    last-prompted: "2021-05-06T00:14:50-07:00"
kubeContexts: []

c) subsequent invocations don't display the update check messages.

➜  ../../out/skaffold version && ../../out/skaffold build -d gcr.io/tejal-gke1 
v1.22.0

Generating tags...
 - base -> gcr.io/tejal-gke1/base:v1.23.0-36-gbf79f0788
 - leeroy-app -> gcr.io/tejal-gke1/leeroy-app:v1.23.0-36-gbf79f0788
 - leeroy-web -> gcr.io/tejal-gke1/leeroy-web:v1.23.0-36-gbf79f0788
Checking cache...
 - base: Found Remotely
 - leeroy-app: Found Remotely
 - leeroy-web: Found Remotely

d) Change the date to yesterday and make sure update message is displayed

global:
  survey:
    last-prompted: "2021-04-27T13:32:59-07:00"
  collect-metrics: true
  update-config:
    last-prompted: "2021-05-05T00:14:50-07:00".   ## yesterday
kubeContexts: []

➜   ../../out/skaffold build -d gcr.io/tejal-gke1 
Generating tags...
 - base -> gcr.io/tejal-gke1/base:v1.23.0-36-gbf79f0788
 - leeroy-app -> gcr.io/tejal-gke1/leeroy-app:v1.23.0-36-gbf79f0788
 - leeroy-web -> gcr.io/tejal-gke1/leeroy-web:v1.23.0-36-gbf79f0788
Checking cache...
 - base: Found Remotely
 - leeroy-app: Found Remotely
 - leeroy-web: Found Remotely
There is a new version (1.23.0) of Skaffold available. Download it from:
  https://github.com/GoogleContainerTools/skaffold/releases/tag/v1.23.0

@tejal29 tejal29 requested a review from a team as a code owner May 6, 2021 07:18
@tejal29 tejal29 requested a review from gsquared94 May 6, 2021 07:18
@google-cla google-cla bot added the cla: yes label May 6, 2021
@tejal29 tejal29 enabled auto-merge (squash) May 6, 2021 07:19
@tejal29 tejal29 changed the title Show update check to users once in 24 hours Show update check message to users once in 24 hours May 6, 2021
@tejal29 tejal29 force-pushed the show_update_check_dailu branch 2 times, most recently from 8a34a00 to 0e97ef8 Compare May 6, 2021 07:44
@tejal29 tejal29 force-pushed the show_update_check_dailu branch from 0e97ef8 to 315b24e Compare May 6, 2021 07:46
@codecov
Copy link

codecov bot commented May 6, 2021

Codecov Report

Merging #5795 (210cd8b) into master (ce34201) will decrease coverage by 0.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5795      +/-   ##
==========================================
- Coverage   70.95%   70.93%   -0.02%     
==========================================
  Files         439      438       -1     
  Lines       16408    16434      +26     
==========================================
+ Hits        11642    11658      +16     
- Misses       3913     3920       +7     
- Partials      853      856       +3     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/cmd.go 63.03% <0.00%> (-0.78%) ⬇️
pkg/skaffold/config/util.go 65.85% <70.00%> (+0.44%) ⬆️
pkg/skaffold/update/update.go 48.38% <100.00%> (ø)
cmd/skaffold/app/cmd/runner.go 58.49% <0.00%> (ø)
pkg/skaffold/runner/v2/runner.go 0.00% <0.00%> (ø)
pkg/skaffold/runner/cleanup.go
pkg/skaffold/runner/apply.go
pkg/skaffold/runner/portforwarder.go
pkg/skaffold/runner/runner.go
pkg/skaffold/runner/tester.go
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce34201...210cd8b. Read the comment docs.

@tejal29 tejal29 added the kokoro:force-run forces a kokoro re-run on a PR label May 6, 2021
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label May 6, 2021
@MarlonGamez
Copy link
Contributor

will do some manual testing and get back 👍🏼

Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! just a couple nits/q's

pkg/skaffold/config/global_config.go Outdated Show resolved Hide resolved
pkg/skaffold/config/global_config.go Outdated Show resolved Hide resolved
Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍🏼

@tejal29 tejal29 merged commit 36395cc into GoogleContainerTools:master May 6, 2021
@tejal29 tejal29 deleted the show_update_check_dailu branch July 21, 2021 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants