Enhancements in clarity to access-application-cluster/connecting-frontend-backend.md #25927

murillodigital · 2021-01-03T22:39:54Z

…ecting-frontend-backend.md

k8s-ci-robot · 2021-01-03T22:40:01Z

Welcome @murillodigital!

It looks like this is your first PR to kubernetes/website 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/website has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

murillodigital · 2021-01-03T22:44:39Z

@Aut0R3V @sftim I've made changes to the overall document towards more accurate description and better clarity, including renaming the manifests and config files to better match their place inside the tutorial. I do have one question, the nginx config in the docs is already baked inside an already published dockerimage gcr.io/google-samples/hello-frontend:1.0 the change to service name and related change to match in the nginx config would effectively be a breaking change for anybody applying the new manifests using the existing dockerimage. How do we go about publishing a new version to match the documentation?

netlify · 2021-01-03T22:45:37Z

✔️ Deploy preview for kubernetes-io-master-staging ready!

🔨 Explore the source changes: 542e4a0

🔍 Inspect the deploy logs: https://app.netlify.com/sites/kubernetes-io-master-staging/deploys/5ff9d6310e08c10008630bbc

😎 Browse the preview: https://deploy-preview-25927--kubernetes-io-master-staging.netlify.app

tengqm · 2021-01-04T00:43:52Z

content/en/docs/tasks/access-application-cluster/connecting-frontend-backend.md

 ```

-At this point, you have a backend Deployment running, and you have a
-Service that can route traffic to it.
+At this point, you have a `backend` Deployment running seven replicas of your `hello`


Sounds to me that 7 replicas is an overkill. Can/shall we reduce it to 3?

Agreed, reduced to 3 (here as well as deployment manifest and sample apply output)

tengqm · 2021-01-04T00:48:43Z

content/en/docs/tasks/access-application-cluster/connecting-frontend-backend.md

-The frontend connects to the backend worker Pods by using the DNS name
-given to the backend Service. The DNS name is "hello", which is the value
-of the `name` field in the preceding Service configuration file.
+Now that you have your backend running, you can create a frontend that is available 


"available" may be a little ambiguous?

Suggested change

Now that you have your backend running, you can create a frontend that is available

Now that you have your backend running, you can create a frontend that is accessible from

tengqm · 2021-01-04T00:50:34Z

content/en/docs/tasks/access-application-cluster/connecting-frontend-backend.md

+Similar to the backend, the frontend has a Deployment and a Service. An important
+difference to notice between the backend and frontend services, is that the
+configuration for the frontend Service has `type: LoadBalancer`, which means that
+the Service uses the default load balancer of your cloud provider and will be


Suggested change

the Service uses the default load balancer of your cloud provider and will be

the Service uses a load balancer of your cloud provider and will be accessible from outside the cluster.

tengqm · 2021-01-04T00:55:16Z

the change to service name and related change to match in the nginx config would effectively be a breaking change for anybody applying the new manifests using the existing dockerimage.

Will the new "nginx.conf" override the defaults in the docker image? If not, this problem may become a blocker.

murillodigital · 2021-01-04T01:10:38Z

the change to service name and related change to match in the nginx config would effectively be a breaking change for anybody applying the new manifests using the existing dockerimage.

Will the new "nginx.conf" override the defaults in the docker image? If not, this problem may become a blocker.

The Dockerfile does indeed replace the default conf file with our frontend-nginx.conf:

RUN rm /etc/nginx/conf.d/default.conf
COPY frontend-nginx.conf /etc/nginx/conf.d

The problem is, the current image will attempt to proxy traffic to a hostname named hello, which with the introduced changes no longer exists (the new backend service name is backend)

tengqm · 2021-01-04T01:39:37Z

The problem is, the current image will attempt to proxy traffic to a hostname named hello, which with the introduced changes no longer exists (the new backend service name is backend)

In that case, we may need to change the backend service name back to "hello"?

murillodigital · 2021-01-04T01:48:46Z

The problem is, the current image will attempt to proxy traffic to a hostname named hello, which with the introduced changes no longer exists (the new backend service name is backend)

In that case, we may need to change the backend service name back to "hello"?

Either that or replace the current image or update one with a newer version, which is what I would prefer.

I don't think the name hello is ideal to properly communicate what the hostname is pointing at, nor consistent with the rest of the nomenclature (backend vs frontend vs app=hello).

My preferred approach is build/push a new image gcr.io/google-samples/hello-frontend:1.1 to match the changes in this PR. and update the deployment manifest accordingly - for that, I would need somebody with push access to the google-samples gcr repository.

tengqm · 2021-01-04T01:52:36Z

My preferred approach is build/push a new image gcr.io/google-samples/hello-frontend:1.1 to match the changes in this PR.

Doable, but we need to find the person for the image upgrade and we will need to hold this PR till the new image is published.

murillodigital · 2021-01-04T01:55:34Z

My preferred approach is build/push a new image gcr.io/google-samples/hello-frontend:1.1 to match the changes in this PR.

Doable, but we need to find the person for the image upgrade and we will need to hold this PR till the new image is published.

Patience is a virtue 🙂. How would I escalate/get a hold of somebody with access and cycles to support? Is there something other than delay that I may be overlooking?

tengqm · 2021-01-04T02:00:23Z

Just tried to google the source for this image but failed. From gcr, I can see the image was created in 2016 and never updated since its initial version. Anyway, I'm okay with upgrading the image too, so long the whole example works.

tengqm · 2021-01-04T02:02:37Z

Is this the source code? https://github.com/GoogleCloudPlatform/kubernetes-engine-samples/tree/master/hello-app

murillodigital · 2021-01-04T02:06:36Z

Is this the source code? https://github.com/GoogleCloudPlatform/kubernetes-engine-samples/tree/master/hello-app

You don't need to change the actual hello-app, only the nginx frontend. The Dockerfile to build it is in the docs repo itself, it's nothing but an nginx image with a swapped config file.

The Dockerfile is at /content/en/examples/service/access - you'd just have to build it from that path with a tag gcr.io/google-samples/hello-frontend:1.1 and push it. (Likely obvious comment but just in case --- of course using the Dockerfile on this PR, not on the current master of the kubernetes/website repo)

tengqm · 2021-01-04T02:24:03Z

@murillodigital I see. Thanks.

murillodigital · 2021-01-08T20:37:55Z

@murillodigital I see. Thanks.

Hi there @tengqm ! Just doing a quick drive by to see if you had been able to make some progress on the image building? Let me know if there's anything I can do to help out. Hope you have a phenomenal weekend!

tengqm · 2021-01-09T01:56:19Z

@murillodigital Unfortunately, I don't have access to the image repo.

murillodigital · 2021-01-09T02:07:53Z

@murillodigital Unfortunately, I don't have access to the image repo.

Not a problem, do you think it's reasonably possible to find somebody with access, or would you advise that, although less than ideal from my perspective, we simply revert the service name (all other changes would still apply)?

tengqm · 2021-01-09T02:53:56Z

@murillodigital I don't know who has this access. Maybe keep the service name as hello is a simpler alternative.

murillodigital · 2021-01-09T16:22:15Z

@murillodigital I don't know who has this access. Maybe keep the service name as hello is a simpler alternative.

@tengqm - Cool, just renamed the backend service back to hello and updated references for consistency, fixed some wording around the term looking to keep clarity - there are no blockers to merging anymore.

murillodigital · 2021-01-14T01:39:29Z

@tengqm - quick ping on this, this should be good to go with my last change.

tengqm · 2021-01-14T01:45:39Z

/label tide/merge-method-squash

tengqm · 2021-01-14T01:45:50Z

/lgtm
/approve

k8s-ci-robot · 2021-01-14T01:46:05Z

LGTM label has been added.

Git tree hash: 3af30a92194097a89b8e65c113c1ef8aac8d2aa2

k8s-ci-robot · 2021-01-14T01:46:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tengqm

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~content/en/OWNERS~~ [tengqm]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Enhancements in clarity to docs/tasks/access-application-cluster/conn…

9fac890

…ecting-frontend-backend.md

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 3, 2021

k8s-ci-robot requested review from celestehorgan and rajeshdeshpande02 January 3, 2021 22:40

k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 3, 2021

tengqm reviewed Jan 4, 2021

View reviewed changes

murillodigital added 2 commits January 4, 2021 01:19

Merge branch 'master' of github.com:kubernetes/website

cc932b8

Code review comments

9278846

Merge branch 'master' of github.com:kubernetes/website

bfbb82d

Reverting name of service due to backed value in Dockerimage

542e4a0

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jan 14, 2021

k8s-ci-robot assigned tengqm Jan 14, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 14, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 14, 2021

k8s-ci-robot merged commit f7b2971 into kubernetes:master Jan 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancements in clarity to access-application-cluster/connecting-frontend-backend.md #25927

Enhancements in clarity to access-application-cluster/connecting-frontend-backend.md #25927

murillodigital commented Jan 3, 2021

k8s-ci-robot commented Jan 3, 2021

murillodigital commented Jan 3, 2021

netlify bot commented Jan 3, 2021 •

edited

Loading

tengqm Jan 4, 2021

murillodigital Jan 4, 2021

tengqm Jan 4, 2021

tengqm Jan 4, 2021 •

edited

Loading

tengqm commented Jan 4, 2021

murillodigital commented Jan 4, 2021

tengqm commented Jan 4, 2021

murillodigital commented Jan 4, 2021

tengqm commented Jan 4, 2021

murillodigital commented Jan 4, 2021

tengqm commented Jan 4, 2021

tengqm commented Jan 4, 2021

murillodigital commented Jan 4, 2021 •

edited

Loading

tengqm commented Jan 4, 2021

murillodigital commented Jan 8, 2021

tengqm commented Jan 9, 2021

murillodigital commented Jan 9, 2021

tengqm commented Jan 9, 2021

murillodigital commented Jan 9, 2021 •

edited

Loading

murillodigital commented Jan 14, 2021

tengqm commented Jan 14, 2021

tengqm commented Jan 14, 2021

k8s-ci-robot commented Jan 14, 2021

k8s-ci-robot commented Jan 14, 2021

	Now that you have your backend running, you can create a frontend that is available
	Now that you have your backend running, you can create a frontend that is accessible from

	the Service uses the default load balancer of your cloud provider and will be
	the Service uses a load balancer of your cloud provider and will be accessible from outside the cluster.

Enhancements in clarity to access-application-cluster/connecting-frontend-backend.md #25927

Enhancements in clarity to access-application-cluster/connecting-frontend-backend.md #25927

Conversation

murillodigital commented Jan 3, 2021

k8s-ci-robot commented Jan 3, 2021

murillodigital commented Jan 3, 2021

netlify bot commented Jan 3, 2021 • edited Loading

tengqm Jan 4, 2021

Choose a reason for hiding this comment

murillodigital Jan 4, 2021

Choose a reason for hiding this comment

tengqm Jan 4, 2021

Choose a reason for hiding this comment

tengqm Jan 4, 2021 • edited Loading

Choose a reason for hiding this comment

tengqm commented Jan 4, 2021

murillodigital commented Jan 4, 2021

tengqm commented Jan 4, 2021

murillodigital commented Jan 4, 2021

tengqm commented Jan 4, 2021

murillodigital commented Jan 4, 2021

tengqm commented Jan 4, 2021

tengqm commented Jan 4, 2021

murillodigital commented Jan 4, 2021 • edited Loading

tengqm commented Jan 4, 2021

murillodigital commented Jan 8, 2021

tengqm commented Jan 9, 2021

murillodigital commented Jan 9, 2021

tengqm commented Jan 9, 2021

murillodigital commented Jan 9, 2021 • edited Loading

murillodigital commented Jan 14, 2021

tengqm commented Jan 14, 2021

tengqm commented Jan 14, 2021

k8s-ci-robot commented Jan 14, 2021

k8s-ci-robot commented Jan 14, 2021

netlify bot commented Jan 3, 2021 •

edited

Loading

tengqm Jan 4, 2021 •

edited

Loading

murillodigital commented Jan 4, 2021 •

edited

Loading

murillodigital commented Jan 9, 2021 •

edited

Loading