Skip to content

Add clusterID to x ps #36835

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

Merged
merged 4 commits into from
Apr 8, 2022
Merged

Conversation

hanxiaop
Copy link
Member

Please provide a description of this PR:
#36290 adds the id to istioctl ps, and this one adds to x ps

To help us figure out who should review this PR, please put an X in all the areas that this PR affects.

  • Configuration Infrastructure
  • Docs
  • Installation
  • Networking
  • Performance and Scalability
  • Policies and Telemetry
  • Security
  • Test and Release
  • User Experience
  • Developer Infrastructure

Please check any characteristics that apply to this pull request.

  • Does not have any user-facing changes. This may include CLI changes, API changes, behavior changes, performance improvements, etc.

@hanxiaop hanxiaop requested review from a team as code owners January 14, 2022 07:06
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 14, 2022
@hanxiaop hanxiaop requested review from a team as code owners January 14, 2022 07:09
@hanxiaop
Copy link
Member Author

/test integ-pilot-istiodremote-tests_istio

@hanxiaop
Copy link
Member Author

@esnible PTAL.

@zirain
Copy link
Member

zirain commented Jan 24, 2022

/test all

@@ -176,8 +178,13 @@ func (s *XdsStatusWriter) setupStatusPrint(drs map[string]*xdsapi.DiscoveryRespo
}
cds, lds, eds, rds := getSyncStatus(&clientConfig)
cp := multixds.CpInfo(dr)
meta, err := model.ParseMetadata(clientConfig.GetNode().GetMetadata())
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to get the clusterID via similar way from getSyncStatus , just like get cds, lds, eds, rds above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Those sync statues are generated from the GenericXdsConfigs in clientConfig, and clusterID, proxyID are from the Node info in the clientConfig. I think they should not be mixed up, but there could be an improvement in the next to unify them to a function like getProxyStatuses.

Copy link
Member Author

Choose a reason for hiding this comment

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

@zhlsunshine PTAL, thanks!

@istio-policy-bot istio-policy-bot added the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label Mar 12, 2022
@hanxiaop hanxiaop removed the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label Mar 12, 2022
@istio-policy-bot istio-policy-bot added the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label Mar 12, 2022
@hanxiaop
Copy link
Member Author

not stale

@istio-policy-bot istio-policy-bot removed the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label Mar 16, 2022
@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 7, 2022
@hanxiaop hanxiaop force-pushed the add-cluster-to-istioctlxps branch from 578ac22 to 11a8b50 Compare April 8, 2022 01:43
@istio-testing istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Apr 8, 2022
@hanxiaop hanxiaop force-pushed the add-cluster-to-istioctlxps branch from 11a8b50 to 557a6a8 Compare April 8, 2022 01:55
@hanxiaop hanxiaop force-pushed the add-cluster-to-istioctlxps branch from 557a6a8 to b95b6d3 Compare April 8, 2022 02:08
@hanxiaop
Copy link
Member Author

hanxiaop commented Apr 8, 2022

/test integ-security-multicluster_istio

@istio-testing istio-testing merged commit a177d8b into istio:master Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants