Skip to content

Conversation

kebe7jun
Copy link
Member

@kebe7jun kebe7jun commented Jan 14, 2021

#29302

[X] User Experience
[X] Developer Infrastructure

[X] Does not have any changes that may affect Istio users.

@kebe7jun kebe7jun requested a review from a team as a code owner January 14, 2021 05:27
@google-cla google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Jan 14, 2021
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 14, 2021
@istio-testing istio-testing added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 14, 2021
@kebe7jun
Copy link
Member Author

/test integ-telemetry-mc-k8s-tests_istio

@kebe7jun kebe7jun changed the title Add DeploymentConflictingPorts analyze Add DeploymentConflictingPorts analyzer Jan 14, 2021
- name: "DeploymentConflictingPorts"
code: IST0137
level: Warning
description: "If we have two services, both selecting the same workload, with the same target port, they should be the same port."
Copy link
Contributor

Choose a reason for hiding this comment

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

Reword without "if"? Perhaps "Two services selecting the same workload with same target port are MUST refer to the same port."

code: IST0137
level: Warning
description: "If we have two services, both selecting the same workload, with the same target port, they should be the same port."
template: "This deployment %s is associated with multiple services %v using targetPort %s but different ports: %v."
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: use %q instead of %s if port name can be empty.

@kebe7jun
Copy link
Member Author

@esnible Thanks, updated.

@kebe7jun
Copy link
Member Author

@esnible Do i need to do something else?

@kebe7jun
Copy link
Member Author

/retest

@istio-testing istio-testing merged commit fa98a3c into istio:master Jan 22, 2021
@kebe7jun kebe7jun deleted the feature/service-conflicting-ports branch January 22, 2021 15:50
@ericvn
Copy link
Contributor

ericvn commented Jan 26, 2021

@kebe7jun @esnible Should these be cherry-picked to release 1.9?

@esnible
Copy link
Contributor

esnible commented Jan 26, 2021

@ericvn What do you think, @kebe7jun ? Usually I do not push hard to get analyzers to cherry-pick, as they are not functional and users can download a daily build of istioctl. If there is 0% chance of the new analyzer incorrectly offering false positive warnings we can cherry pick it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/environments area/test and release area/user experience cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. release-notes-none Indicates a PR that does not require release notes. 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.

5 participants