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.

Sign up for GitHub

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

Jump to bottom

Expose etcd version. #35896

Merged
merged 1 commit into from
Nov 2, 2016
Merged

Expose etcd version. #35896

merged 1 commit into from
Nov 2, 2016

Conversation

wojtek-t
Copy link
Member

@wojtek-t wojtek-t commented Oct 31, 2016 •
edited
Loading

Ref #20504

This PRs makes use of the following two env variables

  • ETCD_IMAGE (if set) allows to use a custom etcd image.
  • ETCD_VERSION (if set) allows you to use custom version of etcd. The main purpose of using it may be rollback of etcd v3 API, where we need 3.0.* image, but are rolling back to 2.3.7.

This change is Reviewable

@wojtek-t wojtek-t added the release-note-none Denotes a PR that doesn't merit a release note. label Oct 31, 2016
@wojtek-t wojtek-t added this to the v1.4 milestone Oct 31, 2016
@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 31, 2016
@wojtek-t
Copy link
Member Author

@k8s-bot test this, issue: #IGNORE (Unable to get latest version from build tree. Exiting...)

@gmarek gmarek closed this Oct 31, 2016
@gmarek gmarek reopened this Oct 31, 2016
mml
mml suggested changes Oct 31, 2016
View reviewed changes
Copy link
Contributor

@mml mml left a comment

Choose a reason for hiding this comment

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

I think we need to explain, either in the PR or commit message (or both) how we're changing the semantics of ETCD_VERSION and why we needed to introduce the new ETCD_IMAGE variable.

It would also be good to document the meaning of the two variables somewhere in the code.

cluster/common.sh
@@ -702,6 +702,10 @@ NUM_NODES: $(yaml-quote ${NUM_NODES})
STORAGE_BACKEND: $(yaml-quote ${STORAGE_BACKEND:-})
ENABLE_GARBAGE_COLLECTOR: $(yaml-quote ${ENABLE_GARBAGE_COLLECTOR:-})
EOF
if [ -n "${ETCD_IMAGE:-}" ]; then
cat >>$file <<EOF
ETCD_IMAGE: $(yaml-quota ${ETCD_IMAGE})
Copy link
Contributor

Choose a reason for hiding this comment

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

s/quota/quote

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@wojtek-t
Copy link
Member Author

wojtek-t commented Nov 2, 2016

@mml - comments applied, PTAL

@wojtek-t
Copy link
Member Author

wojtek-t commented Nov 2, 2016

@kubernetes/test-infra-maintainers - I'm getting the following error in all suites:

Unable to get latest version from build tree. Exiting...

anything I can do with it (I have no idea what is happening - retesting doesn't help).

@mtaufen
Copy link
Contributor

mtaufen commented Nov 2, 2016

Interesting, I ran into this same issue yesterday on #32530, but things are ok on that PR now. It's been 6 hours since these failed, try re-running?

@wojtek-t
Copy link
Member Author

wojtek-t commented Nov 2, 2016

@k8s-bot test this, issue: #IGNORE (Unable to get latest version from build tree. Exiting...)

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit e528542. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit e528542. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit e528542. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit e528542. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit e528542. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit e528542. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@wojtek-t
Copy link
Member Author

wojtek-t commented Nov 2, 2016

@mtaufen - it didn't help

@mtaufen
Copy link
Contributor

mtaufen commented Nov 2, 2016

Hmmmm. This is where that error comes from: https://github.com/kubernetes/release/blame/master/push-build.sh#L104

@mtaufen
Copy link
Contributor

mtaufen commented Nov 2, 2016

Try running cluster/kubectl.sh version --client and paste the output here to see if it matches: http://regexr.com/3eij0

@wojtek-t
Copy link
Member Author

wojtek-t commented Nov 2, 2016

@mtaufen - how can I run it? It's working for me locally.

ixdy
ixdy reviewed Nov 2, 2016
View reviewed changes
cluster/common.sh
@@ -702,6 +702,14 @@ NUM_NODES: $(yaml-quote ${NUM_NODES})
STORAGE_BACKEND: $(yaml-quote ${STORAGE_BACKEND:-})
ENABLE_GARBAGE_COLLECTOR: $(yaml-quote ${ENABLE_GARBAGE_COLLECTOR:-})
EOF
# ETCD_IMAGE (if set) allows to use a custom etcd image.
if [ -n "${ETCD_IMAGE:-}" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

this appears to be missing the fi.

Copy link
Member Author

Choose a reason for hiding this comment

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

heh - I must have added it after last test. but the error is really misleading btw

Copy link
Member

Choose a reason for hiding this comment

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

I agree. I'll fix it.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks!

@mtaufen
Copy link
Contributor

mtaufen commented Nov 2, 2016

Ok. Just wanted to make sure nothing super strange was going on.

@mml mml added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 2, 2016
@mml
Copy link
Contributor

mml commented Nov 2, 2016

LGTM

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 09a4262 into kubernetes:master Nov 2, 2016
@ixdy ixdy mentioned this pull request Nov 2, 2016
Merged
@wojtek-t wojtek-t deleted the target_version branch November 23, 2016 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ixdy ixdy ixdy left review comments

@mml mml mml approved these changes

Assignees

@mml mml

@gmarek gmarek

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@wojtek-t @mtaufen @k8s-ci-robot @mml @k8s-github-robot @ixdy @gmarek @googlebot