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

Add agent hostNetwork option #1257

Merged
merged 1 commit into from
Oct 19, 2020
Merged

Conversation

faceair
Copy link
Contributor

@faceair faceair commented Oct 14, 2020

Add the hostNetwork option to the agent DaemonSet.

Fixes #676.

@faceair
Copy link
Contributor Author

faceair commented Oct 14, 2020

I can't get past the make security check, and now the code of the master branch is broken. For example, the type of the clientset reference doesn't exist, and the generated directory is empty.

I'm confused as to why other people's PRs can do the build.

@jpkrohling
Copy link
Contributor

and now the code of the master branch is broken

It shouldn't be broken, CI is working for it, at least for the last-1 commit. Are you unable to build master locally? If so, you might have different tools/setup than what's expected.

@faceair
Copy link
Contributor Author

faceair commented Oct 14, 2020

Now here's why my CI build is failing: https://github.com/jaegertracing/jaeger-operator/pull/1257/checks?check_run_id=1253291006
My local make ci failed for the same reason.

@jpkrohling
Copy link
Contributor

Are you unable to build master locally?

@faceair
Copy link
Contributor Author

faceair commented Oct 14, 2020

Yes, I can't build master locally, but it looks like it's for a different reason.

> go version
go version go1.15.2 darwin/amd64

> operator-sdk version
operator-sdk version: "v0.18.2", commit: "f059b5e17447b0bbcef50846859519340c17ffad", kubernetes version: "v1.18.2", go version: "go1.13.10 darwin/amd64"

> git log -1
commit 5271b85d99f2861a5e2ae69d4b466feb0b8bd63f (HEAD -> master, origin/master, origin/HEAD)
Author: Kevin Earls <kearls@redhat.com>
Date:   Tue Oct 13 16:13:22 2020 +0200

    Add tests for OTEL ingester (#1252)

    Signed-off-by: Kevin Earls <kearls@redhat.com>

> make ci
INFO[0000] Running deepcopy code-generation for Custom Resource group versions: [jaegertracing:[v1], kafka:[v1beta1], ]
F1014 20:52:49.984684    3973 deepcopy.go:922] Hit an unsupported type invalid type for *invalid type
Failed to generate the Kubernetes stubs.
make: *** [internal-generate] Error 255

> # after add GOROOT in makefile

> make ci
INFO[0000] Running deepcopy code-generation for Custom Resource group versions: [jaegertracing:[v1], kafka:[v1beta1], ]
INFO[0037] Code-generation complete.
Formatting code...
pkg/client/versioned/clientset.go
pkg/client/versioned/fake/clientset_generated.go
pkg/client/versioned/fake/register.go
pkg/client/versioned/scheme/register.go
Build failed: the versioned clients aren't up to date. Run 'make generate'.
make: *** [ensure-generate-is-noop] Error 1

> make generate
INFO[0000] Running deepcopy code-generation for Custom Resource group versions: [jaegertracing:[v1], kafka:[v1beta1], ]
INFO[0033] Code-generation complete.
Formatting code...
pkg/client/versioned/clientset.go
pkg/client/versioned/fake/clientset_generated.go
pkg/client/versioned/fake/register.go
pkg/client/versioned/scheme/register.go

> make ci
INFO[0000] Running deepcopy code-generation for Custom Resource group versions: [jaegertracing:[v1], kafka:[v1beta1], ]
INFO[0034] Code-generation complete.
Formatting code...
pkg/client/versioned/clientset.go
pkg/client/versioned/fake/clientset_generated.go
pkg/client/versioned/fake/register.go
pkg/client/versioned/scheme/register.go
Build failed: the versioned clients aren't up to date. Run 'make generate'.
make: *** [ensure-generate-is-noop] Error 1

Also, I can't complete the make security check on master.

> make security
Security...
Results:

Golang errors in file: [/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/client/versioned/clientset.go]:

  > [line 17 : column 36] - JaegertracingV1Interface not declared by package v1

  > [line 24 : column 35] - JaegertracingV1Client not declared by package v1

  > [line 28 : column 55] - JaegertracingV1Interface not declared by package v1

  > [line 53 : column 44] - NewForConfig not declared by package v1

  > [line 69 : column 39] - NewForConfigOrDie not declared by package v1

  > [line 78 : column 39] - New not declared by package v1


Golang errors in file: [/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/client/versioned/fake/clientset_generated.go]:

  > [line 65 : column 55] - JaegertracingV1Interface not declared by package v1

  > [line 66 : column 30] - FakeJaegertracingV1 not declared by package fake


Golang errors in file: [/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/client/versioned/fake/register.go]:

  > [line 19 : column 18] - AddToScheme not declared by package v1


Golang errors in file: [/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/client/versioned/scheme/register.go]:

  > [line 19 : column 18] - AddToScheme not declared by package v1



[/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/storage/elasticsearch_secrets.go:208] - G307 (CWE-): Deferring unsafe method "*os.File" on type "Close" (Confidence: HIGH, Severity: MEDIUM)
  > defer f.Close()


[/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/cmd/generate/main.go:49] - G307 (CWE-): Deferring unsafe method "*os.File" on type "Close" (Confidence: HIGH, Severity: MEDIUM)
  > defer f.Close()


[/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/cmd/generate/main.go:89] - G307 (CWE-): Deferring unsafe method "*os.File" on type "Close" (Confidence: HIGH, Severity: MEDIUM)
  > defer out.Close()


[/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/test/e2e/utils.go:313] - G306 (CWE-): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
  > ioutil.WriteFile(logFileName, log, 0644)


Summary:
   Files: 143
   Lines: 17296
   Nosec: 6
  Issues: 4

make: *** [security] Error 1

On my own branch I just can't get the make security check done, it's almost identical to the CI results.

> make ci
INFO[0000] Running deepcopy code-generation for Custom Resource group versions: [jaegertracing:[v1], kafka:[v1beta1], ]
INFO[0036] Code-generation complete.
Formatting code...
pkg/client/versioned/clientset.go
pkg/client/versioned/fake/clientset_generated.go
pkg/client/versioned/fake/register.go
pkg/client/versioned/scheme/register.go
Checking...
Linting...
Security...
Results:

Golang errors in file: [/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/client/versioned/clientset.go]:

  > [line 17 : column 36] - JaegertracingV1Interface not declared by package v1

  > [line 24 : column 35] - JaegertracingV1Client not declared by package v1

  > [line 28 : column 55] - JaegertracingV1Interface not declared by package v1

  > [line 53 : column 44] - NewForConfig not declared by package v1

  > [line 69 : column 39] - NewForConfigOrDie not declared by package v1

  > [line 78 : column 39] - New not declared by package v1


Golang errors in file: [/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/client/versioned/fake/clientset_generated.go]:

  > [line 65 : column 55] - JaegertracingV1Interface not declared by package v1

  > [line 66 : column 30] - FakeJaegertracingV1 not declared by package fake


Golang errors in file: [/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/client/versioned/fake/register.go]:

  > [line 19 : column 18] - AddToScheme not declared by package v1


Golang errors in file: [/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/client/versioned/scheme/register.go]:

  > [line 19 : column 18] - AddToScheme not declared by package v1



[/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/cmd/generate/main.go:49] - G307 (CWE-): Deferring unsafe method "*os.File" on type "Close" (Confidence: HIGH, Severity: MEDIUM)
  > defer f.Close()


[/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/cmd/generate/main.go:89] - G307 (CWE-): Deferring unsafe method "*os.File" on type "Close" (Confidence: HIGH, Severity: MEDIUM)
  > defer out.Close()


[/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/test/e2e/utils.go:313] - G306 (CWE-): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
  > ioutil.WriteFile(logFileName, log, 0644)


[/Users/faceair/Developer/Go/src/github.com/jaegertracing/jaeger-operator/pkg/storage/elasticsearch_secrets.go:208] - G307 (CWE-): Deferring unsafe method "*os.File" on type "Close" (Confidence: HIGH, Severity: MEDIUM)
  > defer f.Close()


Summary:
   Files: 143
   Lines: 17369
   Nosec: 6
  Issues: 4

make: *** [security] Error 1

I just switched branches, I didn't do anything else.

@jpkrohling
Copy link
Contributor

Then there's certainly something odd with your setup, as master indeed works in CI:

image

@faceair
Copy link
Contributor Author

faceair commented Oct 14, 2020

I found that the version of gosec used in CI in the latest commit is v0.0.0-20191008095658-28c1128b7336
And the version of gosec my PR's CI uses is v0.0.0-20200401082031-e946c8c39989
Same script, getting a different version of the tool.

I now have gosec locked to v0.0.0-20191008095658-28c1128b7336. But it looks like I'm having other problems, so I'll keep working on that.

@codecov
Copy link

codecov bot commented Oct 14, 2020

Codecov Report

Merging #1257 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1257      +/-   ##
==========================================
+ Coverage   87.36%   87.37%   +0.01%     
==========================================
  Files          90       90              
  Lines        4907     4911       +4     
==========================================
+ Hits         4287     4291       +4     
  Misses        457      457              
  Partials      163      163              
Impacted Files Coverage Δ
pkg/apis/jaegertracing/v1/jaeger_types.go 100.00% <ø> (ø)
pkg/deployment/agent.go 96.21% <100.00%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5271b85...654b118. Read the comment docs.

@faceair
Copy link
Contributor Author

faceair commented Oct 14, 2020

It looks like some of the issues are related to the fact that go get will modify the project's go.mod file, and I isolated the tools dependency via vogt.sh.

Hi @jpkrohling , please help review if there are any other issues!

@jpkrohling
Copy link
Contributor

jpkrohling commented Oct 15, 2020

That's wonderful! The vgot script looks nice and might be interesting to @ibraimgm as well.

Would you be able to split this PR into two, one for each feature this one here is bringing?

  1. fix the dependencies (vgot changes + GOROOT)
  2. add the hostNetwork. You'll also need to add a unit test for it.

You'll also need to handle the DCO failure.

@faceair faceair mentioned this pull request Oct 16, 2020
@faceair faceair changed the title add common hostNetwork option add agent hostNetwork option Oct 16, 2020
Signed-off-by: faceair <git@faceair.me>
@jpkrohling jpkrohling changed the title add agent hostNetwork option Add agent hostNetwork option Oct 19, 2020
@mergify mergify bot merged commit cb02287 into jaegertracing:master Oct 19, 2020
mergify bot pushed a commit that referenced this pull request Oct 19, 2020
Previous discussions #1257
@faceair
Copy link
Contributor Author

faceair commented Oct 19, 2020

Hi @jpkrohling
What is the planned release of jaeger-operator? How soon can I use this feature?

@faceair faceair deleted the add_host_network branch October 19, 2020 11:33
@jpkrohling
Copy link
Contributor

We typically release new versions of the operator following new versions of Jaeger, which is typically released once every two months. The last release was about 20 days ago.

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.

Agent daemonset should be started with hostNetwork
3 participants