GoogleCloudPlatform / k8s-multicluster-ingress

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

Rewrite zone-printer app #165

Closed ahmetb closed 6 years ago

ahmetb commented 6 years ago

~~This will likely require a lot of README.md changes, so please hold off until the other PRs regarding tutorial updates are merged.~~

Signed-off-by: Ahmet Alp Balkan ahmetb@google.com

ahmetb commented 6 years ago

I plan to make this image available on gcr.io/google-samples/zone-printer as soon as possible.

Preview screenshots:

screen shot 2018-04-02 at 10 53 04 am

This is from geopeeker, I have only 2 datacenters (EU and US) but normally you'd see a lot more flags.

screen shot 2018-04-02 at 10 58 37 am
nikhiljindal commented 6 years ago

Thanks for the PR @ahmetb The example looks so much cooler with country flags :)

Note that since you are renaming and moving around files, you will also need to update all instances pointing to these files. We do that in the README (kubeclt create -f app/) and in our tests (test/e2e).

ahmetb commented 6 years ago

@nikhiljindal Any ideas why the e2e tests didn't fail here?

nikhiljindal commented 6 years ago

We dont have them running on ci yet. There are different tests running on ci: https://k8s-testgrid.appspot.com/sig-multicluster-kubemci#kubemci-ingress-conformance. The code for these is in k/k.

ahmetb commented 6 years ago

I'll go through test/* and update stuff once these are merged. Does this sound good?

I'm also not sure about the exact structure of YAML files, so I may do another shuffling of files.

nikhiljindal commented 6 years ago

Can you do that in this same PR? That way we ensure that our example is working all the time. Dont want to keep the example in a broken state

ahmetb commented 6 years ago

Depending on the usage volume of the project, as long as my PRs are reviewed I don't think it's a concern. I respond on GitHub very fast and can put together a fixing PR really quickly.

Currently it happened to be that tutorial rewrite and sample rewrite are two separate PRs, because I wasn't expecting to do both, but ended up doing so.

I recommend you merge my PRs fast I will be providing fixes. :) No need to turn this into a logistics problem.

ahmetb commented 6 years ago

Now that #163 is merged, I can actually probably fix all this in this PR. /hold

ahmetb commented 6 years ago

/hold cancel

@nikhiljindal made the changes in 86ccc7db. I'm not sure how to run the tests though.

nikhiljindal commented 6 years ago

go run /test/e2e/e2e.go It expects you to have a kubeconfig file named minKubeconfig with your clusters. For a simple sanity check, you can have a single cluster in your kubeconfig.

nikhiljindal commented 6 years ago

Thanks for the changes. /lgtm

nikhiljindal commented 6 years ago

Let me know when you feel you are ready and I will merge this

ahmetb commented 6 years ago

@nikhiljindal I just pushed another commit correcting the image: field, but the e2e tests have been running for a very long time now with no output (I can see the status of the mci and visit its IP, it works fine).

Finally, they failed with this:

E0404 14:54:01.330381    5807 basic.go:69] error in waiting for ingress: Failed to execute a successful GET within 12m0s, Last response body for http:///, host :

timed out waiting for the condition
PASS: got 200 from ingress url
F0404 14:54:02.469170    5807 basic.go:129] Status does not contain lb name (wialesqrmi) and IP (): NAME         IP               CLUSTERS
jwouajiaue   35.186.252.223   gke_kubemci-2018_us-central1-b_c1
goroutine 1 [running]:
github.com/GoogleCloudPlatform/k8s-multicluster-ingress/vendor/github.com/golang/glog.stacks(0xc420398700, 0xc42039e000, 0xcb, 0x227)
    /Users/ahmetb/workspace/gopath-k8s-multicluster-ingress/src/github.com/GoogleCloudPlatform/k8s-multicluster-ingress/vendor/github.com/golang/glog/glog.go:766 +0xcf
github.com/GoogleCloudPlatform/k8s-multicluster-ingress/vendor/github.com/golang/glog.(*loggingT).output(0x32cd0a0, 0xc400000003, 0xc42012c580, 0x324763c, 0x8, 0x81, 0x0)
    /Users/ahmetb/workspace/gopath-k8s-multicluster-ingress/src/github.com/GoogleCloudPlatform/k8s-multicluster-ingress/vendor/github.com/golang/glog/glog.go:717 +0x30f
github.com/GoogleCloudPlatform/k8s-multicluster-ingress/vendor/github.com/golang/glog.(*loggingT).printf(0x32cd0a0, 0xc400000003, 0x2645996, 0x34, 0xc4204cfd98, 0x3, 0x3)
    /Users/ahmetb/workspace/gopath-k8s-multicluster-ingress/src/github.com/GoogleCloudPlatform/k8s-multicluster-ingress/vendor/github.com/golang/glog/glog.go:655 +0x14b
github.com/GoogleCloudPlatform/k8s-multicluster-ingress/vendor/github.com/golang/glog.Fatalf(0x2645996, 0x34, 0xc4204cfd98, 0x3, 0x3)
    /Users/ahmetb/workspace/gopath-k8s-multicluster-ingress/src/github.com/GoogleCloudPlatform/k8s-multicluster-ingress/vendor/github.com/golang/glog/glog.go:1145 +0x67
github.com/GoogleCloudPlatform/k8s-multicluster-ingress/test/e2e/cases.testList(0xc420350ec0, 0xc, 0x0, 0x0, 0xc420046f00, 0xa)
    /Users/ahmetb/workspace/gopath-k8s-multicluster-ingress/src/github.com/GoogleCloudPlatform/k8s-multicluster-ingress/test/e2e/cases/basic.go:129 +0x41c
github.com/GoogleCloudPlatform/k8s-multicluster-ingress/test/e2e/cases.testHTTPIngress(0xc420350ec0, 0xc, 0x2613170, 0xd, 0xc420046f00, 0xa)
    /Users/ahmetb/workspace/gopath-k8s-multicluster-ingress/src/github.com/GoogleCloudPlatform/k8s-multicluster-ingress/test/e2e/cases/basic.go:72 +0x2c8
github.com/GoogleCloudPlatform/k8s-multicluster-ingress/test/e2e/cases.RunBasicCreateTest()
    /Users/ahmetb/workspace/gopath-k8s-multicluster-ingress/src/github.com/GoogleCloudPlatform/k8s-multicluster-ingress/test/e2e/cases/basic.go:37 +0x2ad
main.main()
    /Users/ahmetb/workspace/gopath-k8s-multicluster-ingress/src/github.com/GoogleCloudPlatform/k8s-multicluster-ingress/test/e2e/e2e.go:25 +0x25
exit status 255

which I'm not sure if it's related to my changes.

I think it's because I CTRL+C'ed the previous test run and now there are artifacts of it.

nikhiljindal commented 6 years ago

@ahmetb Were you able to resolve the issue?

You seem to have gotten far enough the setup that the app and ingress seem to have been created correctly, indicating that the paths are working fine.

Have added one comment about the image name. mostly lgtm

ahmetb commented 6 years ago

yes, I got it to pass. It appears like when you terminate it halfway through, something is causing a problem. I tried it on a clean cluster.

$ go run test/e2e/e2e.go

PASS: got 200 from ingress url
PASS: found loadbalancer name and IP in 'list' command.
PASS: got 200 from ingress url
PASS: found loadbalancer name and IP in 'list' command.
PASS: health check path matches path from readiness probe
nikhiljindal commented 6 years ago

Cool, ready to merge?

ahmetb commented 6 years ago

Yes. :shipit: