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

operator: Fix default API server addr in metrics subcommand #26132

Merged

Conversation

pippolo84
Copy link
Member

The operator API server has been modularized into its independent cell in a previous commit.

Doing that, the flag to specify a custom address for the server has been moved from the global operator config to the new cell config.

The metrics list subcommand needs to know the operator API server address to query the "/metrics" endpoint. Before the modularization, the subcommand was at least able to query the server if that was started at the default address.

This commit adds a new flag to specify the address of the operator API server. Also, the default value for this flag is set to the same default used in the operator/api server cell. Doing that fixes the bug introduced in the modularization and also allows to specify another address if the operator API server is listening on a non-default one.

Fixes: 11d5856 ("operator/api: Modularize api server")

@pippolo84 pippolo84 added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/metrics Impacts statistics / metrics gathering, eg via Prometheus. area/operator Impacts the cilium-operator component release-blocker/1.14 This issue will prevent the release of the next version of Cilium. affects/v1.14 This issue affects v1.14 branch labels Jun 12, 2023
@pippolo84 pippolo84 requested a review from a team as a code owner June 12, 2023 16:52
@pippolo84 pippolo84 requested a review from tklauser June 12, 2023 16:52
The operator API server has been modularized into its independent cell
in a previous commit.

Doing that, the flag to specify a custom address for the server has been
moved from the global operator config to the new cell config.

The `metrics list` subcommand needs to know the operator API server
address to query the "/metrics" endpoint.  Before the modularization,
the subcommand was at least able to query the server if that was started
at the default address.

This commit adds a new flag to specify the address of the operator API
server. Also, the default value for this flag is set to the same default
used in the operator/api server cell.  Doing that fixes the bug
introduced in the modularization and also allows to specify another
address if the operator API server is listening on a non-default one.

Fixes: 11d5856 ("operator/api: Modularize api server")

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
@pippolo84 pippolo84 force-pushed the pr/pippolo84/fix-operator-metrics-list branch from dd4d3ac to 113d8bf Compare June 12, 2023 17:15
@tklauser tklauser removed the affects/v1.14 This issue affects v1.14 branch label Jun 12, 2023
@tklauser
Copy link
Member

tklauser commented Jun 12, 2023

/test

Job 'Cilium-PR-K8s-1.26-kernel-net-next' hit: #25958 (89.10% similarity)

@pippolo84
Copy link
Member Author

Conformance E2E failure tracked in #26114, rerunning.

@pippolo84
Copy link
Member Author

/test-1.26-net-next

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 13, 2023
@joestringer joestringer merged commit 0377ffa into cilium:main Jun 14, 2023
63 of 65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Impacts statistics / metrics gathering, eg via Prometheus. area/operator Impacts the cilium-operator component ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants