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

Extend cilium-operator binary to be used as command line tool #14484

Merged
merged 6 commits into from
Jan 7, 2021

Conversation

fristonio
Copy link
Member

@fristonio fristonio commented Dec 23, 2020

This PullRequest aims at extending the cilium-operator binary to be used as a command-line tool. There are two reworks associated with this PR.

  1. Make cilium-operator to use openapi config as apiserver implementation similar to what we have for Cilium and cilium-health.
  2. Add metric subcommand to cilium-operator to list metrics from a running cilium-operator instance apiserver.

Fixes #13132

image

@fristonio fristonio added the area/operator Impacts the cilium-operator component label Dec 23, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 23, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Dec 23, 2020
@fristonio fristonio added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Dec 23, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 23, 2020
@fristonio fristonio force-pushed the pr/fristonio/operator-api-and-cmd branch 2 times, most recently from 8454d52 to f68651a Compare December 23, 2020 15:07
@fristonio fristonio marked this pull request as ready for review December 23, 2020 15:15
@fristonio fristonio requested a review from a team December 23, 2020 15:15
@fristonio fristonio requested review from a team as code owners December 23, 2020 15:15
@fristonio fristonio requested a review from a team December 23, 2020 15:15
@fristonio fristonio requested a review from a team as a code owner December 23, 2020 15:15
@fristonio
Copy link
Member Author

test-me-please

@fristonio
Copy link
Member Author

retest-gke

Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

TIL about go-swagger generation 👍

Couple of comments I have as below, will do another round of review later.

api/v1/operator/openapi.yaml Outdated Show resolved Hide resolved
api/v1/operator/openapi.yaml Outdated Show resolved Hide resolved
operator/main.go Show resolved Hide resolved
operator/cmd/root.go Show resolved Hide resolved
@fristonio fristonio force-pushed the pr/fristonio/operator-api-and-cmd branch 2 times, most recently from 4f4e858 to 181b76d Compare January 5, 2021 11:41
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

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

Overall LGTM. I think CODEOWNERS should be updated to take into account the new /api/v1/operator directory:

diff --git a/CODEOWNERS b/CODEOWNERS
index bdbe06c83..29d6a492e 100644
--- a/CODEOWNERS
+++ b/CODEOWNERS
@@ -35,6 +35,7 @@
 /api/ @cilium/api
 /api/v1/flow/ @cilium/api @cilium/hubble
 /api/v1/observer/ @cilium/api @cilium/hubble
+/api/v1/operator/ @cilium/api @cilium/operator
 /api/v1/peer/ @cilium/api @cilium/hubble
 /bpf/ @cilium/bpf
 Makefile* @cilium/build

@fristonio fristonio requested a review from a team as a code owner January 5, 2021 12:17
@kaworu kaworu removed their assignment Jan 5, 2021
@fristonio
Copy link
Member Author

test-me-please

api/v1/operator/openapi.yaml Outdated Show resolved Hide resolved
operator/api/server.go Outdated Show resolved Hide resolved
@fristonio fristonio force-pushed the pr/fristonio/operator-api-and-cmd branch from e47d1f5 to b738a1d Compare January 6, 2021 07:31
Signed-off-by: Deepesh Pathak <deepshpathak@gmail.com>
Signed-off-by: Deepesh Pathak <deepshpathak@gmail.com>
Signed-off-by: Deepesh Pathak <deepshpathak@gmail.com>
* By default the open-api spec contains the /v1 basepath. This commit
  adds a separate handler for /healthz requests outside of the default
  basepath. This is done using HTTP server mux where the index handler
  is the default open-api router and `/healthz` requests are handled by
  a new handler.

Signed-off-by: Deepesh Pathak <deepshpathak@gmail.com>
Signed-off-by: Deepesh Pathak <deepshpathak@gmail.com>
Signed-off-by: Deepesh Pathak <deepshpathak@gmail.com>
@fristonio fristonio force-pushed the pr/fristonio/operator-api-and-cmd branch from b738a1d to e40cf73 Compare January 6, 2021 08:49
@fristonio
Copy link
Member Author

test-me-please

Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

LGTM 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operator Impacts the cilium-operator component release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Expose cilium-operator metrics
7 participants