Service port names should follow istio-naming convention? #5070

iamejboy · 2019-08-06T09:18:33Z

Service port name to istio-naming convention #5018
https://istio.io/docs/setup/kubernetes/additional-setup/requirements/

Fixes #

Proposed Changes

Service port name should follow the istio naming convention based on this link:
https://istio.io/docs/setup/kubernetes/additional-setup/requirements/

Note: Please update if I missed something but the idea is there.

Release Note

* Service port name should follow the istio naming convention based on this link:
https://istio.io/docs/setup/kubernetes/additional-setup/requirements/

knative-prow-robot

@iamejboy: 0 warnings.

In response to this:

Service port name to istio-naming convention #5018
https://istio.io/docs/setup/kubernetes/additional-setup/requirements/

Fixes #

Proposed Changes

Service port name should follow the istio naming convention based on this link:
https://istio.io/docs/setup/kubernetes/additional-setup/requirements/

Note: Please update if I missed something but the idea is there.

Release Note
* Service port name should follow the istio naming convention based on this link:
https://istio.io/docs/setup/kubernetes/additional-setup/requirements/

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

knative-prow-robot · 2019-08-06T09:18:48Z

Hi @iamejboy. Thanks for your PR.

I'm waiting for a knative 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.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dgerd · 2019-08-06T16:17:04Z

/ok-to-test

markusthoemmes

Do we potentially need to touch activator-service.yaml and controller-service.yaml as well?

pkg/apis/serving/v1alpha1/revision_lifecycle.go

config/autoscaler-service.yaml

pkg/apis/serving/v1alpha1/revision_lifecycle.go

tcnghia · 2019-08-14T06:34:00Z

/test pull-knative-serving-istio-1.1-mesh
/test pull-knative-serving-istio-1.2-mesh

markusthoemmes · 2019-08-21T10:25:15Z

@iamejboy any news on this? Had any chance to look at the test failures yet?

iamejboy · 2019-08-21T14:34:29Z

@iamejboy any news on this? Had any chance to look at the test failures yet?

Hi @markusthoemmes, Sorry, didn't have time yet or might be needing your help to get this merge. I link an issue from istio, reason why we are strictly enforcing this internally.

iamejboy · 2019-08-28T01:21:09Z

/retest

iamejboy · 2019-09-01T00:35:30Z

Hi @markusthoemmes updated PR. Test looks good now. cc: @dgerd

markusthoemmes

/lgtm
/approve

markusthoemmes

/hold

Shot too quick. One question left.

markusthoemmes · 2019-09-09T08:29:12Z

pkg/apis/serving/v1alpha1/revision_lifecycle.go

@@ -52,11 +52,11 @@ const (

 	// UserQueueMetricsPortName specifies the port name to use for metrics
 	// emitted by queue-proxy for end user.
-	UserQueueMetricsPortName = "user-metrics"
+	UserQueueMetricsPortName = "http-usermetric"


@mattmoor is this acceptable from an update PoV? I feel like, because these names are used when generating the deployment, we effectively trigger a redeployment when this change is applied after an update.

We should be able to safely rollout changes to the user Deployment

cc @tcnghia

Ack. I think this may be ok. Following the conventions may help us avoid strange errors.

markusthoemmes · 2019-09-09T08:29:45Z

/cc @mattmoor

tcnghia · 2019-09-20T00:32:45Z

/hold cancel
/approve

tcnghia · 2019-09-20T04:06:13Z

/assign @mattmoor

markusthoemmes · 2019-10-28T08:48:12Z

@mattmoor this is awaiting input from your end. Any chance to get to this?

mattmoor · 2019-10-28T13:55:21Z

This looks good to me, but let's land it after the cut Tuesday out of an abundance of caution.

…/istio.io/docs/setup/kubernetes/additional-setup/requirements/

markusthoemmes

/lgtm
/approve

@mattmoor this might need your superpowers.

mattmoor

/approve

knative-prow-robot · 2019-11-15T14:42:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: iamejboy, markusthoemmes, mattmoor, tcnghia

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:

~~OWNERS~~ [mattmoor]

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

knative-prow-robot · 2019-11-15T14:47:50Z

@iamejboy: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-knative-serving-istio-1.1-mesh	`78704a6`	link	`/test pull-knative-serving-istio-1.1-mesh`
pull-knative-serving-istio-1.2-mesh	`78704a6`	link	`/test pull-knative-serving-istio-1.2-mesh`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

knative-test-reporter-robot · 2019-11-15T14:47:56Z

The following jobs failed:

Test name	Triggers	Retries
pull-knative-serving-unit-tests	pull-knative-serving-unit-tests	1/3

Automatically retrying due to test flakiness...
/test pull-knative-serving-unit-tests

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Aug 6, 2019

knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 6, 2019

knative-prow-robot reviewed Aug 6, 2019

View reviewed changes

knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 6, 2019

knative-prow-robot requested review from dgerd and mattmoor August 6, 2019 09:18

knative-prow-robot added the area/API API objects and controllers label Aug 6, 2019

mattmoor assigned tcnghia Aug 6, 2019

mattmoor removed their request for review August 6, 2019 14:17

knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 6, 2019

dgerd added area/networking and removed area/API API objects and controllers labels Aug 6, 2019

markusthoemmes reviewed Aug 7, 2019

View reviewed changes

pkg/apis/serving/v1alpha1/revision_lifecycle.go Outdated Show resolved Hide resolved

config/autoscaler-service.yaml Show resolved Hide resolved

knative-prow-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/API API objects and controllers and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 8, 2019

markusthoemmes reviewed Aug 8, 2019

View reviewed changes

pkg/apis/serving/v1alpha1/revision_lifecycle.go Outdated Show resolved Hide resolved

iamejboy force-pushed the service_port_name_update branch from 78704a6 to 3b0f6dd Compare August 25, 2019 01:37

knative-prow-robot added the area/autoscale label Aug 25, 2019

iamejboy force-pushed the service_port_name_update branch from 3b0f6dd to aac0669 Compare September 1, 2019 00:05

markusthoemmes approved these changes Sep 9, 2019

View reviewed changes

knative-prow-robot assigned markusthoemmes Sep 9, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2019

markusthoemmes reviewed Sep 9, 2019

View reviewed changes

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 9, 2019

knative-prow-robot requested a review from mattmoor September 9, 2019 08:29

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 20, 2019

knative-prow-robot assigned mattmoor Sep 20, 2019

update service ports name following istio-naming convention - https:/…

479e7af

…/istio.io/docs/setup/kubernetes/additional-setup/requirements/

iamejboy force-pushed the service_port_name_update branch from aac0669 to 3bf352f Compare November 11, 2019 01:58

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2019

update missed and rename service port's name

2a04b3b

iamejboy force-pushed the service_port_name_update branch from 3bf352f to 2a04b3b Compare November 15, 2019 01:55

markusthoemmes approved these changes Nov 15, 2019

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 15, 2019

mattmoor reviewed Nov 15, 2019

View reviewed changes

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 15, 2019

knative-prow-robot merged commit 6e247eb into knative:master Nov 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Service port names should follow istio-naming convention? #5070

Service port names should follow istio-naming convention? #5070

iamejboy commented Aug 6, 2019

knative-prow-robot left a comment

knative-prow-robot commented Aug 6, 2019

dgerd commented Aug 6, 2019

markusthoemmes left a comment

tcnghia commented Aug 14, 2019

markusthoemmes commented Aug 21, 2019

iamejboy commented Aug 21, 2019

iamejboy commented Aug 28, 2019

iamejboy commented Sep 1, 2019

markusthoemmes left a comment

markusthoemmes left a comment

markusthoemmes Sep 9, 2019

mattmoor Sep 17, 2019

mattmoor Sep 17, 2019

tcnghia Sep 20, 2019

markusthoemmes commented Sep 9, 2019

tcnghia commented Sep 20, 2019

tcnghia commented Sep 20, 2019

markusthoemmes commented Oct 28, 2019

mattmoor commented Oct 28, 2019

markusthoemmes left a comment

mattmoor left a comment

knative-prow-robot commented Nov 15, 2019

knative-prow-robot commented Nov 15, 2019 •

edited

Loading

knative-test-reporter-robot commented Nov 15, 2019

Service port names should follow istio-naming convention? #5070

Service port names should follow istio-naming convention? #5070

Conversation

iamejboy commented Aug 6, 2019

Proposed Changes

knative-prow-robot left a comment

Choose a reason for hiding this comment

Proposed Changes

knative-prow-robot commented Aug 6, 2019

dgerd commented Aug 6, 2019

markusthoemmes left a comment

Choose a reason for hiding this comment

tcnghia commented Aug 14, 2019

markusthoemmes commented Aug 21, 2019

iamejboy commented Aug 21, 2019

iamejboy commented Aug 28, 2019

iamejboy commented Sep 1, 2019

markusthoemmes left a comment

Choose a reason for hiding this comment

markusthoemmes left a comment

Choose a reason for hiding this comment

markusthoemmes Sep 9, 2019

Choose a reason for hiding this comment

mattmoor Sep 17, 2019

Choose a reason for hiding this comment

mattmoor Sep 17, 2019

Choose a reason for hiding this comment

tcnghia Sep 20, 2019

Choose a reason for hiding this comment

markusthoemmes commented Sep 9, 2019

tcnghia commented Sep 20, 2019

tcnghia commented Sep 20, 2019

markusthoemmes commented Oct 28, 2019

mattmoor commented Oct 28, 2019

markusthoemmes left a comment

Choose a reason for hiding this comment

mattmoor left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Nov 15, 2019

knative-prow-robot commented Nov 15, 2019 • edited Loading

knative-test-reporter-robot commented Nov 15, 2019

knative-prow-robot commented Nov 15, 2019 •

edited

Loading