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

hive: Add hive.Command() #23074

Merged
merged 4 commits into from Jan 27, 2023
Merged

hive: Add hive.Command() #23074

merged 4 commits into from Jan 27, 2023

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Jan 12, 2023

hive.Command() now implements the hive related cobra commands, so they don't need to be repeated per binary. See commits for more details.

$ go run ./daemon hive
Cells:

  Ⓜ️ agent (Cilium Agent):
      Ⓜ️ infra (Infrastructure):
          Ⓜ️ gops (Gops Agent):
              ⚙️ (gops.GopsConfig) {
                  GopsPort: (uint16) 9890
              }
...

$ go run ./operator hive
Cells:
...

$ go run ./daemon hive dot-graph | dot -Tx11

@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 Jan 12, 2023
@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label Jan 12, 2023
@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 Jan 12, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jan 12, 2023
Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@joamaki joamaki marked this pull request as ready for review January 12, 2023 16:06
@joamaki joamaki requested review from a team as code owners January 12, 2023 16:06
@joamaki
Copy link
Contributor Author

joamaki commented Jan 12, 2023

/test

@joamaki joamaki force-pushed the pr/joamaki/hive-commands branch 5 times, most recently from f90f738 to fcd4641 Compare January 16, 2023 11:23
Remove endpoint-gc-interval flag from option.Config as it already
was in endpointmanager.Config. Looks like use of PersistentFlags()
allowed duplicates. PersistentFlags no longer needed as the hive
sub-command now takes the hive's flag set (so it doesn't need to inherit
from root command).

Signed-off-by: Jussi Maki <jussi@isovalent.com>
Implement the commands for hive object and dot-graph dumping
centrally, so we'll avoid duplicate implementations in each
binary. Add the hive as sub-command to agent, operator and
clustermesh-apiserver.

Add a test for the agent cell to check that it can be instantiated
with the default configuration so we don't break the ability to
inspect the agent hive.

Remove endpoint-gc-interval flag from option.Config as it already
was in endpointmanager.Config. Looks like use of PersistentFlags()
allowed duplicates. PersistentFlags no longer needed as the hive
sub-command now takes the hive's flag set (so it doesn't need to inherit
from root command).

Signed-off-by: Jussi Maki <jussi@isovalent.com>
LBIPAM nil-deref'd when k8s configuration was not provided, fix
by adding a nil check. While at it, move lbipam cell definitions
to cell.go.

Add test to operator to check that the operator hive can be populated.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
The line wrapping was still broken when there were multiple wrapped lines,
since after wrapping the current line length was miscalculated.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki
Copy link
Contributor Author

joamaki commented Jan 17, 2023

/test

Job 'Cilium-PR-K8s-1.16-kernel-4.9' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks E/W loadbalancing (ClusterIP, NodePort from inside cluster, etc) Checks service on same node

Failure Output

FAIL: Found 1 k8s-app=cilium logs matching list of errors that must be investigated:

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.16-kernel-4.9 so I can create one.

@joamaki joamaki requested a review from tklauser January 26, 2023 11:03
@tklauser tklauser removed the kind/community-contribution This was a contribution made by a community member. label Jan 26, 2023
@joamaki joamaki added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 26, 2023
@aanm aanm merged commit 35b35be into cilium:master Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants