Skip to content

[Draft]Convert bash script to GoLang - #590

Open
kapjain-rh wants to merge 4 commits into
netobserv:mainfrom
kapjain-rh:netobserv-1414
Open

kapjain-rh wants to merge 4 commits into
netobserv:mainfrom
kapjain-rh:netobserv-1414

Conversation

@kapjain-rh

Copy link
Copy Markdown
Member

Description

Dependencies

n/a

Checklist

  • Does the changes in PR need specific configuration or environment set up for testing?
    • if so please describe it in PR description.
  • I have added thorough unit tests for the change.
  • QE requirements (check 1 from the list):
    • Standard QE validation, with pre-merge tests unless stated otherwise.
    • Regression tests only (e.g. refactoring with no user-facing change).
    • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

@openshift-ci

openshift-ci Bot commented Sep 24, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign oliviercazade for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Kapil Jain <kapjain@redhat.com>
Signed-off-by: Kapil Jain <kapjain@redhat.com>
Signed-off-by: Kapil Jain <kapjain@redhat.com>
Signed-off-by: Kapil Jain <kapjain@redhat.com>
@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 55.57391% with 538 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.79%. Comparing base (d2afe56) to head (1a7cbd6).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
internal/pkg/plugin/cluster.go 24.50% 212 Missing and 16 partials ⚠️
internal/pkg/plugin/command.go 42.50% 54 Missing and 15 partials ⚠️
internal/pkg/plugin/copy.go 49.45% 31 Missing and 15 partials ⚠️
internal/pkg/plugin/options.go 71.97% 26 Missing and 18 partials ⚠️
e2e/cluster/kind.go 0.00% 29 Missing ⚠️
internal/pkg/plugin/manifests.go 86.53% 14 Missing and 14 partials ⚠️
internal/pkg/plugin/metrics_port.go 66.12% 10 Missing and 11 partials ⚠️
cmd/packet_capture.go 61.36% 15 Missing and 2 partials ⚠️
internal/pkg/plugin/serve.go 58.97% 13 Missing and 3 partials ⚠️
cmd/flow_capture.go 50.00% 11 Missing and 2 partials ⚠️
... and 5 more
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #590       +/-   ##
===========================================
+ Coverage   21.47%   32.79%   +11.31%     
===========================================
  Files          26       36       +10     
  Lines        3115     4269     +1154     
===========================================
+ Hits          669     1400      +731     
- Misses       2374     2680      +306     
- Partials       72      189      +117     
Flag Coverage Δ
unittests 32.79% <55.57%> (+11.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/root.go 47.36% <87.50%> (+10.91%) ⬆️
e2e/common.go 0.00% <0.00%> (ø)
internal/pkg/plugin/plaintext.go 92.50% <92.50%> (ø)
cmd/oc-netobserv/main.go 0.00% <0.00%> (ø)
internal/pkg/plugin/stdin.go 67.56% <67.56%> (ø)
cmd/flow_capture.go 36.78% <50.00%> (+29.18%) ⬆️
internal/pkg/plugin/serve.go 58.97% <58.97%> (ø)
cmd/packet_capture.go 29.13% <61.36%> (+26.72%) ⬆️
internal/pkg/plugin/metrics_port.go 66.12% <66.12%> (ø)
internal/pkg/plugin/manifests.go 86.53% <86.53%> (ø)
... and 5 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jpinsonneau jpinsonneau left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would restructure files to avoid having everything under internal/pkg/plugin. Something like:

internal/
  cli/          # Public commands, argument compatibility, prompts
  capture/      # Capture requests, sessions, orchestration
  kube/         # Kubernetes clients, deployment, exec, logs
  manifests/    # Resource and pipeline construction
  collector/    # Receiving records, limits, processing
  artifacts/    # JSON, SQLite, PCAPNG, JSONL, transfer
  terminal/     # Interactive views and presentation
  tlsresolver/  # Existing TLS profile resolution

WDYT ?

Comment thread Makefile

GOLANGCI_LINT_VERSION = v2.12.2
BASH_VERSION = v4.2.0
YQ_VERSION = v4.45.1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we still need YQ ? We should handle everything in golang

Comment on lines +16 to +25
type option struct{ key, value string }
type options struct {
raw []string
mode, namespace, kubeconfig, context, output, copy, logLevel string
maxTime time.Duration
maxBytes int64
background, headless, yaml, subnets bool
values []option
filters []map[string]any
}

@jpinsonneau jpinsonneau Sep 24, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That deserve a real golang type like

type CaptureRequest struct {
    Kind       CaptureKind
    Connection ConnectionOptions
    Execution  ExecutionOptions
    Agent      AgentConfig
    Filters    []FilterGroup
    Queries    []string
    Limits     CaptureLimits
    Output     OutputOptions
}

We should try to reuse as most as possible types from operator there

@openshift-ci

openshift-ci Bot commented Sep 29, 2026

Copy link
Copy Markdown

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants