Skip to content

have downloadIstioCandidate.sh ask github for the latest#1977

Merged
istio-merge-robot merged 4 commits intoistio:masterfrom
mattdelco:master3
Jan 7, 2018
Merged

have downloadIstioCandidate.sh ask github for the latest#1977
istio-merge-robot merged 4 commits intoistio:masterfrom
mattdelco:master3

Conversation

@mattdelco
Copy link
Copy Markdown
Contributor

This change allows downloadIstioCandidate.sh to automatically ask github for the latest version. It seems to work on Mac and Linux and I've preserved the behavior to allow ISTIO_VERSION to override the version. This file isn't to be confused with ../downloadIstio.sh that's used to grab the latest stable.

The main tradeoff I can think of (besides if I've unknowlying added an inappropriate dependency) is that if we have a release like 0.3.0 and then do a new release for a patch update (e.g., a hypothetical 0.2.15) then github might report that 0.2.15 is the latest. I'm not sure I have a good solution for this. Even if I have the script take the max(github-latest, ISTIO_VERSION-in-file) then this won't be ideal either if the version in the script is stale, and my eventual hope/goal would be to get rid of the baked-in version (and the need to update it).

Thoughts on whether this is a worthwhile or achievable goal?

NONE

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you shouldn't need a temp file, can't you just chain the grep,... with pipes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I got started with a file from initially using cut, then again from parsing out two pieces of data (the version and the URL--I couldn't find a decent way to try to split stdout into two pipes). If I only grab the version and continue with the pre-existing approach of using a URL template than I can forego the file.

Copy link
Copy Markdown
Member

@ldemailly ldemailly Dec 4, 2017

Choose a reason for hiding this comment

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

you only need 1 pass so you can do it all with awk for instance

| awk ' /version/ {version=$3} /url/ {url=$5} END {print version,url}'

for instance (not actual correct regex or field index number, just an illustration)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe just using the

  "tag_name": "0.3.0",

to get the version is enough (+ checking for

 "draft": false,

)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The doc on the API (https://developer.github.com/v3/repos/releases/#get-the-latest-release) says "Draft releases and prereleases are not returned by this endpoint", so that's why I don't bother to check for draft or pre-release.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

then we just need to get the version and nothing else ?

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 4, 2017

Codecov Report

Merging #1977 into master will decrease coverage by 1.31%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1977      +/-   ##
==========================================
- Coverage   81.24%   79.93%   -1.32%     
==========================================
  Files         215       74     -141     
  Lines       17091     6896   -10195     
==========================================
- Hits        13885     5512    -8373     
+ Misses       2695     1098    -1597     
+ Partials      511      286     -225
Flag Coverage Δ
#broker 46.09% <ø> (ø) ⬆️
#mixer ?
#pilot 80.86% <ø> (+3.6%) ⬆️
#security 88.88% <ø> (+1.64%) ⬆️
Impacted Files Coverage Δ
pilot/platform/kube/inject/http.go 72.09% <0%> (-4.66%) ⬇️
pilot/model/validation.go 92.74% <0%> (-2.29%) ⬇️
pilot/adapter/config/file/monitor.go 79.8% <0%> (-0.2%) ⬇️
pilot/adapter/config/ingress/conversion.go 100% <0%> (ø) ⬆️
pilot/model/conversion.go 88.4% <0%> (ø) ⬆️
mixer/pkg/config/store/queue.go
mixer/pkg/attribute/dictState.go
mixer/pkg/config/runtime.go
mixer/pkg/il/evaluator/checker.go
mixer/adapter/memquota/memquota.go
... and 161 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 24089ea...ce6d764. Read the comment docs.

@ldemailly ldemailly added the do-not-merge Block automatic merging of a PR. label Dec 4, 2017
@ldemailly
Copy link
Copy Markdown
Member

this is good to go except not this week for the same reason as
#1940
(kubecon) - so let's hold on to it for the end of the week

@istio-merge-robot
Copy link
Copy Markdown

@mattdelco PR needs rebase

@istio-merge-robot istio-merge-robot added the needs-rebase Indicates a PR needs to be rebased before being merged label Dec 12, 2017
@istio-merge-robot istio-merge-robot removed the needs-rebase Indicates a PR needs to be rebased before being merged label Dec 14, 2017
@ldemailly ldemailly removed the do-not-merge Block automatic merging of a PR. label Jan 6, 2018
@ldemailly
Copy link
Copy Markdown
Member

/lgtm

@istio-merge-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ldemailly

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link
Copy Markdown

/test all [submit-queue is verifying that this PR is safe to merge]

@ldemailly
Copy link
Copy Markdown
Member

/test istio-pilot-e2e

@istio-merge-robot
Copy link
Copy Markdown

/test all [submit-queue is verifying that this PR is safe to merge]

@istio-merge-robot
Copy link
Copy Markdown

Automatic merge from submit-queue.

@istio-merge-robot istio-merge-robot merged commit fbecb74 into istio:master Jan 7, 2018
@ldemailly
Copy link
Copy Markdown
Member

verified that it worked on my mac:

$ curl -L https://git.io/getLatestIstio |sh -
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  1448  100  1448    0     0   1748      0 --:--:-- --:--:-- --:--:--     0
Downloading istio-0.4.0 from https://github.com/istio/istio/releases/download/0.4.0/istio-0.4.0-osx.tar.gz ...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   612    0   612    0     0    285      0 --:--:--  0:00:02 --:--:--   285
100 10.2M  100 10.2M    0     0   571k      0  0:00:18  0:00:18 --:--:--  720k
Downloaded into istio-0.4.0:
LICENSE		README.md	bin		install		istio.VERSION	samples
Add /Users/ldemailly/tmp/tmp2/istio-0.4.0/bin to your path; e.g copy paste in your shell and/or ~/.profile:
export PATH="$PATH:/Users/ldemailly/tmp/tmp2/istio-0.4.0/bin"
$ ./istio-0.4.0/bin/istioctl version
Version: 0.4.0
GitRevision: 24089ea97c8d244493c93b499a666ddf4010b547-dirty
GitBranch: 6401744b90b43901b2aa4a8bced33c7bd54ffc13
User: root@cc5c34bbd1ee
GolangVersion: go1.8

@mattdelco mattdelco deleted the master3 branch January 11, 2018 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants