GoogleCloudPlatform / k8s-multicluster-ingress

kubemci: Command line tool to configure L7 load balancers using multiple kubernetes clusters
Apache License 2.0
377 stars 68 forks source link

Update GCE cluster version in the README.md #105

Closed kuang-byte closed 6 years ago

kuang-byte commented 6 years ago

Update cluster version to latest "1.8.4.gke-1" since "1.8.0-gke.0" has been removed from the list.

k8s-ci-robot commented 6 years ago

Hi @kuanghaochina. Thanks for your PR.

I'm waiting for a kubernetes or GoogleCloudPlatform member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
nikhiljindal commented 6 years ago

/ok-to-test

Thanks for the update. Why not use the same cluster version everywhere? I see that you are using a different one for europe-west1-c

kuang-byte commented 6 years ago

@nikhiljindal I would like to but I don't know why europe-west1-c doesn't have version 1.8.4.gke-0 and some of the zones under europe does and some of them doesn't. May be europe-west1-c could be updated to europe-west2-c which has 1.8.4.gke-0 instead of 1.8.4.gke-1.

coveralls commented 6 years ago

Coverage Status

Changes Unknown when pulling 808e5f0798b8e98b95519962f4498ca1ccad2a0d on kuanghaochina:patch-1 into on GoogleCloudPlatform:master.

nikhiljindal commented 6 years ago

Thanks @kuanghaochina I would say that these version numbers are for illustrative purposes only. There is no way we can keep them updated all the time.

Just choose any one version and use it in all zones.

kuang-byte commented 6 years ago

@nikhiljindal Thanks for the response. It makes sense. We can close it.

madhusudancs commented 6 years ago

@nikhiljindal @kuanghaochina

I would say update it to a version that's available now (1.8.4-gke.0 in this PR is correct) until the minimum required version (1.8.x+) becomes the default. When 1.8.x+ becomes the default, just remove --cluster-version altogether from the examples.

In short, I would suggest merging this PR.

coveralls commented 6 years ago

Coverage Status

Changes Unknown when pulling 808e5f0798b8e98b95519962f4498ca1ccad2a0d on kuanghaochina:patch-1 into on GoogleCloudPlatform:master.

nikhiljindal commented 6 years ago

Yes, am fine merging this PR but I will prefer selecting a version that works in all zones.

G-Harmon commented 6 years ago

I checked the available versions again today (1/4/18), and now these 3 zones all have 1.8.4-gke.1. You were probably seeing things in the middle of its rollout.

kuang-byte commented 6 years ago

@G-Harmon I think so and I just updated them to the latest version. Thanks for the check.

coveralls commented 6 years ago

Coverage Status

Changes Unknown when pulling 480fd37250f1de4d8ed20f5d8d89da69279bfd19 on kuanghaochina:patch-1 into on GoogleCloudPlatform:master.

nikhiljindal commented 6 years ago

/lgtm, thx!

nikhiljindal commented 6 years ago

aah didnt realize that this had 3 commits. @kuanghaochina for next time, we request squashing commits before merging.

kuang-byte commented 6 years ago

@nikhiljindal em. You are right. I forgot to squash and will pay attention next time. Thanks.