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

ci: Add Hubble helpers #11232

Merged
merged 3 commits into from May 13, 2020
Merged

ci: Add Hubble helpers #11232

merged 3 commits into from May 13, 2020

Conversation

gandro
Copy link
Member

@gandro gandro commented Apr 29, 2020

This PR adds various helpers to make it easier to invoke Hubble from Ginkgo:

helpers.Kubectl:

  • GetHubbleClientPodOnNode() obtains the name of the hubble-cli pod running on a particular node by its name (mirrors GetCiliumPodOnNode`).
  • GetHubbleClientPodOnNodeWithLabel() obtains the name of the hubble-cli running on a node by label (mirrors GetCiliumPodOnNodeWithLabel`).
  • HubbleObserve() runs hubble observe --json on the specified pod and waits for its completion.
  • HubbleObserveFollow() runs hubble observe --json --follow on the specified pod in the background (e.g. to combine with WaitUntilMatchFilterLine() below).

helpers.CmdRes:

  • FilterLines() applies a JSONPath filter to each line of the output and returns its result (similar to the existing Filter method, but supports newline-delimited JSON).
  • WaitUntilMatchFilterLine() applies a JSONPath filter to each line of the output and waits until a line matches a custom string.
  • WaitUntilMatchFilterLineTimeout() same as above, but with a custom timeout.

In addition, a ExpectHubbleClientReady() helper is added and invoked as part of the Cilium deployment, which deployes hubble-cli pods if the global.hubble.cli.enabled Helm option is set.

The existing Hubble skeleton is adapted to use these new helpers.

@gandro gandro added area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/ci This PR makes changes to the CI. sig/hubble Impacts hubble server or relay ci/flake This is a known failure that occurs in the tree. Please investigate me! labels Apr 29, 2020
@gandro gandro requested a review from a team as a code owner April 29, 2020 17:29
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Apr 29, 2020
@gandro gandro requested a review from a team April 29, 2020 17:29
@gandro
Copy link
Member Author

gandro commented Apr 29, 2020

test-me-please

@coveralls
Copy link

coveralls commented Apr 29, 2020

Coverage Status

Coverage increased (+0.01%) to 37.845% when pulling 3f1f6f0 on pr/gandro/ci-hubble-helpers into d613dea on master.

test/helpers/cmd.go Outdated Show resolved Hide resolved
@gandro gandro force-pushed the pr/gandro/ci-hubble-helpers branch from 20ff8dc to d52b6d1 Compare April 30, 2020 09:13
@gandro
Copy link
Member Author

gandro commented Apr 30, 2020

I've removed the ExpectHubbleClientReady helper for now. I thought it could be enabled unconditionally, but this breaks other tests. As enabling Hubble in other test suites is not a priority right now, I've dropped the commit so this PR can get merged sooner.

@gandro
Copy link
Member Author

gandro commented Apr 30, 2020

test-me-please

@gandro gandro marked this pull request as draft May 4, 2020 13:44
@gandro
Copy link
Member Author

gandro commented May 4, 2020

Put back into draft mode, since the follow mode locally has shown flakes due to cilium/hubble#131 (comment)

Edit: This has been fixed.

@gandro gandro added this to In progress in Hubble server May 5, 2020
@gandro gandro force-pushed the pr/gandro/ci-hubble-helpers branch from d52b6d1 to e77d50e Compare May 6, 2020 12:37
@gandro gandro marked this pull request as ready for review May 6, 2020 12:38
@gandro
Copy link
Member Author

gandro commented May 6, 2020

test-me-please

Edit: Hit #10538

@gandro gandro closed this May 6, 2020
@gandro gandro reopened this May 6, 2020
@gandro
Copy link
Member Author

gandro commented May 6, 2020

test-me-please

Copy link
Member

@tgraf tgraf left a comment

Choose a reason for hiding this comment

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

Awesome

res := kub.ExecShort(fmt.Sprintf(
"%s -n %s get pods -l k8s-app=hubble-cli %s", KubectlCmd, namespace, filter))
if !res.WasSuccessful() {
return "", fmt.Errorf("Cilium pod not found on node '%s'", node)
Copy link
Member

Choose a reason for hiding this comment

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

I know it's existing code but would be great to add:

Add : %s", node, res.OutputPrettyPrint())

return false
}

err = WithTimeout(
Copy link
Member

Choose a reason for hiding this comment

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

You might want to use the new WaitUntilTrue()

Copy link
Member Author

@gandro gandro May 7, 2020

Choose a reason for hiding this comment

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

I'm not sure what you are referring to? I did not find any references to WaitUntilTrue?

Edit: Ah, nevermind. I believe you meant RepeatUntilTrue!

ctx, cancel := context.WithTimeout(context.Background(), helpers.MidCommandTimeout)
defer cancel()
follow := kubectl.HubbleObserveFollow(ctx, hubbleNamespace, hubblePodK8s1, fmt.Sprintf(
"--last 1 --type trace --from-pod %s/%s --to-namespace %s --to-label %s --to-port %d",
Copy link
Member

Choose a reason for hiding this comment

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

do you want --last 1 with follow?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is intentional, yes. It's supposed to address a potential race:

The hubble observe --follow process is started in the background. Therefore, there is a small possibility that we proceed with the curl request before the --follow process in the background is actually receiving flows yset. Therefore, --follow would miss the curl request, causing the test to fail. By introducing --last, we don't care which happens first: If the curl request happened before, we can still get it from the ring buffer thanks to --last, if the curl request happend later, then we will receive it thanks to follow-mode.

@gandro gandro force-pushed the pr/gandro/ci-hubble-helpers branch from e77d50e to 8d4710f Compare May 7, 2020 09:29
@gandro gandro requested a review from tgraf May 7, 2020 09:30
@gandro
Copy link
Member Author

gandro commented May 7, 2020

test-me-please

Copy link
Member

@nebril nebril left a comment

Choose a reason for hiding this comment

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

Looks good overall, two comments inline.

test/helpers/cmd.go Outdated Show resolved Hide resolved
// FilterLines works like Filter, but applies the JSONPath filter to each line
// separately and returns returns a FilterBuffer for each line. An error is
// returned only for the first line which cannot be unmarshalled.
func (res *CmdRes) FilterLines(filter string) ([]FilterBuffer, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not yet. However, I expect this to be used in the Hubble tests we will be adding in the upcoming days. If you feel this addition is premature, I can remove it again.

@b3a-dev b3a-dev added this to In Progress (Hubble) in CI Force May 8, 2020
@gandro gandro force-pushed the pr/gandro/ci-hubble-helpers branch from 8d4710f to 7ac2eb6 Compare May 11, 2020 11:28
@gandro
Copy link
Member Author

gandro commented May 11, 2020

test-me-please

1 similar comment
@gandro
Copy link
Member Author

gandro commented May 11, 2020

test-me-please

@gandro gandro requested a review from nebril May 11, 2020 17:12
@gandro
Copy link
Member Author

gandro commented May 12, 2020

test-me-please

This allows tests to connect to a Hubble client running on a specific
node, e.g. to inspect the flows of the cilium-embedded Hubble server.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This commit adds the `HubbleObserve` and `HubbleObserveFollow` helpers
to run `hubble observe` commands via Ginkgo. Support for newline-
delimited JSON is added to `CmdRes`, allowing to apply JSONPath filters
line by line.

`helpers.Kubectl`:

  - `HubbleObserve()` runs `hubble observe --json` on the specified pod
     and waits for its completion.
  - `HubbleObserveFollow()` runs `hubble observe --json --follow` on
     the specified pod in the background
     (e.g. to combine with `WaitUntilMatchFilterLine()` below).

`helpers.CmdRes`:

  - `FilterLines()` applies a JSONPath filter to each line of the output
     and returns its result (similar to the existing `Filter` method,
     but supports newline-delimited JSON).
  - `WaitUntilMatchFilterLine()` applies a JSONPath filter to each line
    of the output and waits until a line matches a custom string.
  - `WaitUntilMatchFilterLineTimeout()` same as above, but with a custom
    timeout.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This switches the Hubble CI skeleton to use the new `--follow` based
helpers and JSONPath filters.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro
Copy link
Member Author

gandro commented May 12, 2020

retest-runtime

Edit: Rebased

@gandro gandro force-pushed the pr/gandro/ci-hubble-helpers branch from 7ac2eb6 to 3f1f6f0 Compare May 12, 2020 13:34
@gandro
Copy link
Member Author

gandro commented May 12, 2020

test-me-please

@gandro
Copy link
Member Author

gandro commented May 12, 2020

retest-4.9

@gandro
Copy link
Member Author

gandro commented May 12, 2020

retest-net-next

1 similar comment
@gandro
Copy link
Member Author

gandro commented May 12, 2020

retest-net-next

@gandro
Copy link
Member Author

gandro commented May 13, 2020

All required tests passed (except generate-k8s-api which is currently known to be stuck).

@gandro gandro merged commit 690905a into master May 13, 2020
1.8.0 automation moved this from In progress to Merged May 13, 2020
Hubble server automation moved this from In progress (1.8) to Done May 13, 2020
@gandro gandro deleted the pr/gandro/ci-hubble-helpers branch May 13, 2020 07:56
CI Force automation moved this from In Progress (Hubble) to Fixed / Done May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow ci/flake This is a known failure that occurs in the tree. Please investigate me! release-note/ci This PR makes changes to the CI. sig/hubble Impacts hubble server or relay
Projects
No open projects
1.8.0
  
Merged
CI Force
  
Fixed / Done
Hubble server
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants