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

Guestbook updates: GCR images, support both dns service and env vars #12497

Merged
merged 1 commit into from
Sep 9, 2015

Conversation

amygdala
Copy link
Contributor

Use GCR images from 'google-samples' project; allow switch on whether dns service is
supported, or to use env vars to get service host info.
Replaces #11787 .

@amygdala
Copy link
Contributor Author

This is towards #9107.

@k8s-bot
Copy link

k8s-bot commented Aug 10, 2015

GCE e2e build/test failed for commit 6243838cec5482d22a1ca8fb37cb5f70cd0e5935.

@k8s-bot
Copy link

k8s-bot commented Aug 10, 2015

GCE e2e build/test failed for commit bfbb9bcd263da6dd46adf6c49d58c8c768bafb69.

@k8s-bot
Copy link

k8s-bot commented Aug 11, 2015

GCE e2e build/test failed for commit 09f559143798ffa320f978c7e7a874d5b3affc26.

@amygdala
Copy link
Contributor Author

The new version of the app, and the new GCR images, work fine for me™. I don't know why the jenkins test is failing. I'll try running the test again in a bit.

@k8s-bot
Copy link

k8s-bot commented Aug 12, 2015

GCE e2e build/test failed for commit 1119aee1404b8e5042735bfa239715f6dcee50f8.

@amygdala
Copy link
Contributor Author

@brendanburns Brendan, the jenkins build has failed enough times that I no longer think it's a fluke. However, nothing jumps out at me from the build log.
Would you have time to see if you can tell what's up? (sorry!) Multiple starts/restarts of the app have all seemed fine to me.
(The tests aren't assuming that the external load balancer for the frontend should be enabled, are they?)

@brendandburns
Copy link
Contributor

@k8s-bot test this please

`GET_HOSTS_FROM` env value in both
`examples/guestbook/redis-slave-controller.yaml` and `examples/guestbook/frontend-controller.yaml`
from `dns` to `env` before you start up the app.
(However, this is unlikely to be necessary. You can check for the DNS service in the list of the clusters' services, e.g. via
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make these instructions more explicit:

kubectl --namespace=kube-system get rc kube-dns or somesuch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@brendandburns
Copy link
Contributor

one minor comment, otherwise lgtm. I pinged Jenkins again.

@k8s-bot
Copy link

k8s-bot commented Aug 13, 2015

GCE e2e build/test failed for commit 1119aee1404b8e5042735bfa239715f6dcee50f8.

@brendandburns
Copy link
Contributor

Needs a rebase.

@brendandburns brendandburns added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2015
@amygdala
Copy link
Contributor Author

Rebased.
I see that there's a campaign to move example images into a GCR 'google_containers' project. My GCR images are in a devrel-run 'google_samples' project. Don't know if that's something we need to reconcile.
My frontend image differs in building from the php:5-apache image (rather than your custom image), as well as supporting the dns vs env stuff.

Let's see what Jenkins does this time. I did double check that the google_samples images were properly accessible from a personal account, and just ran through setting up the app again. All seems fine. If the build continues to fail I might need some assistance digging into why.

@k8s-bot
Copy link

k8s-bot commented Aug 19, 2015

GCE e2e build/test failed for commit 547c44a55cdb3c2014e3aec36a1ad515e5e3f4be.

@brendandburns
Copy link
Contributor

Jenkins is still broken. Are your images publically readable?

@amygdala
Copy link
Contributor Author

Yes, they are readable-- I confirmed that just a regular 'docker pull' from a personal laptop (no gcloud installed etc.) works fine, and got someone else to confirm as well, just to double check.
Something is up though.

@k8s-bot
Copy link

k8s-bot commented Aug 21, 2015

GCE e2e build/test failed for commit b64d386eea5532bdf53c7606fcba4a27c6f3f0a2.

@amygdala
Copy link
Contributor Author

Unfortunately I'll have to punt on looking into this further until next week. Does seem like something's up, but as I noted above, it's all running fine for me. Maybe someone more familiar than me with the Jenkins setup can see if something jumps out at them.
Next week I'll give it a try on a personal account.

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 21, 2015
@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 28, 2015
@k8s-github-robot
Copy link

Labelling this PR as size/L

@k8s-bot
Copy link

k8s-bot commented Aug 31, 2015

GCE e2e build/test failed for commit ddeb284c4c111ab6bb20c8a113690fd574becfa0.

@amygdala
Copy link
Contributor Author

I tried running this example from a personal account as well, to double check the gcr access, and again it worked fine. I think I will need to get some help/consultation to figure out the jenkins issue.

… dns service is

supported, or to use env vars to get service host info.

Test change to reflect php filename change.
@k8s-bot
Copy link

k8s-bot commented Sep 4, 2015

GCE e2e build/test passed for commit 3574999.

@amygdala
Copy link
Contributor Author

amygdala commented Sep 4, 2015

hah! turned out I needed to change a test.

@brendandburns brendandburns added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 8, 2015
@brendandburns
Copy link
Contributor

LGTM, @amygdala thanks for finishing this out.

@k8s-github-robot
Copy link

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

@k8s-bot
Copy link

k8s-bot commented Sep 9, 2015

GCE e2e build/test passed for commit 3574999.

@k8s-github-robot
Copy link

Automatic merge from SubmitQueue

k8s-github-robot pushed a commit that referenced this pull request Sep 9, 2015
Auto commit by PR queue bot
@k8s-github-robot k8s-github-robot merged commit c8526ad into kubernetes:master Sep 9, 2015
@sttts
Copy link
Contributor

sttts commented Sep 9, 2015

It seems that gcr.io/google_samples/gb-frontend:v2 referenced in this PR is not the latest build of the guestbook. The changes of #12199 (comment) are not included.

@amygdala
Copy link
Contributor Author

amygdala commented Sep 9, 2015

@sttts The build files, in the php-redis subdir, have incorporated those changes to the relative URLs -- https://github.com/kubernetes/kubernetes/pull/12497/files (check that those changes look as expected to you).
Let me double check the image build date.
I did change the name of index.php-->guestbook.php (related to a change in the apache setup) and that initially caused one of the Jenkins tests to fail.

@sttts
Copy link
Contributor

sttts commented Sep 9, 2015

$ docker run -it gcr.io/google_samples/gb-frontend:v2 cat index.html                                                                                                     
<html ng-app="redis">
  <head>
    <title>Guestbook</title>
    <link rel="stylesheet" href="//netdna.bootstrapcdn.com/bootstrap/3.1.1/css/bootstrap.min.css">
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.12/angular.min.js"></script>
    <script src="/controllers.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/angular-ui-bootstrap/0.13.0/ui-bootstrap-tpls.js"></script>
  </head>
  <body ng-controller="RedisCtrl">
    <div style="width: 50%; margin-left: 20px">
      <h2>Guestbook</h2>
    <form>
    <fieldset>
    <input ng-model="msg" placeholder="Messages" class="form-control" type="text" name="input"><br>
    <button type="button" class="btn btn-primary" ng-click="controller.onRedis()">Submit</button>
    </fieldset>
    </form>
    <div>
      <div ng-repeat="msg in messages track by $index">
        {{msg}}
      </div>
    </div>
    </div>
  </body>
</html>

Compare the line with controllers.php with the checked in index.html. In the later it's relative.

@amygdala
Copy link
Contributor Author

amygdala commented Sep 9, 2015

@sttts yes, it looks like the image needs to be rebuilt. Sorry about that.
gcr.io/google_samples/gb-frontend:v3 should fix it.
I'll test and submit a PR to bump the image version in the .yaml shortly.

@sttts
Copy link
Contributor

sttts commented Sep 9, 2015

@amygdala thanks for the fast fix 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants