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

#920: Adding the Jaeger client generated code through client-gen #921

Merged
merged 1 commit into from
Mar 11, 2020

Conversation

rareddy
Copy link
Contributor

@rareddy rareddy commented Feb 21, 2020

No description provided.

@rareddy
Copy link
Contributor Author

rareddy commented Feb 21, 2020

Not sure how to mark to ignore the formatting checks in the generated code.

@jpkrohling
Copy link
Contributor

Not sure how to mark to ignore the formatting checks in the generated code.

The trick is not to ignore the formatting checks, but to run make format right after make generate. We do this in OpenTelemetry as well:

.PHONY: generate
generate: internal-generate format

.PHONY: internal-generate
internal-generate:
	@GOPATH=${GOPATH} ./.ci/generate.sh

@rareddy
Copy link
Contributor Author

rareddy commented Feb 24, 2020

Does the jaeger-operator support GOMODULES or build need to be inside the GOPATH?

@rareddy rareddy force-pushed the jaeger_client branch 3 times, most recently from e025273 to b42ec5b Compare February 25, 2020 00:38
@rareddy
Copy link
Contributor Author

rareddy commented Feb 25, 2020

locally when I run make ci it works fine, but not sure what's going on here. Unless there is a version difference in some of the tools with ci build this should not happen now as I checked in the generated and then formatted code that the build is also checking and failing on.

@jpkrohling
Copy link
Contributor

jpkrohling commented Feb 25, 2020

You probably just need to run make format once and update the PR.

See inline comment.

@@ -13,25 +13,25 @@ import (

func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition {
return map[string]common.OpenAPIDefinition{
"./pkg/apis/jaegertracing/v1.ElasticsearchSpec": schema_pkg_apis_jaegertracing_v1_ElasticsearchSpec(ref),
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, those changes are the ones triggering the build failure. If I run make ci locally, I get the same failure as GitHub CI, and this file is shown as the one having changes:

$ make ci
INFO[0000] Running CRD generator.                       
INFO[0001] CRD generation complete.                     
INFO[0000] Running deepcopy code-generation for Custom Resource group versions: [jaegertracing:[v1], kafka:[v1beta1], ] 
INFO[0021] 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: a model has been changed but the generated resources aren't up to date. Run 'make generate' and update your PR.
make: *** [Makefile:41: ensure-generate-is-noop] Error 1

$ git diff
diff --git a/pkg/apis/jaegertracing/v1/zz_generated.openapi.go b/pkg/apis/jaegertracing/v1/zz_generated.openapi.go
index 128461b1..08b11cf5 100644
--- a/pkg/apis/jaegertracing/v1/zz_generated.openapi.go
+++ b/pkg/apis/jaegertracing/v1/zz_generated.openapi.go
@@ -13,25 +13,25 @@ import (
 
 func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition {
        return map[string]common.OpenAPIDefinition{
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.ElasticsearchSpec":               schema_pkg_apis_jaegertracing_v1_ElasticsearchSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.Jaeger":                          schema_pkg_apis_jaegertracing_v1_Jaeger(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerAgentSpec":                 schema_pkg_apis_jaegertracing_v1_JaegerAgentSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerAllInOneSpec":              schema_pkg_apis_jaegertracing_v1_JaegerAllInOneSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerCassandraCreateSchemaSpec": schema_pkg_apis_jaegertracing_v1_JaegerCassandraCreateSchemaSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerCollectorSpec":             schema_pkg_apis_jaegertracing_v1_JaegerCollectorSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerCommonSpec":                schema_pkg_apis_jaegertracing_v1_JaegerCommonSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerDependenciesSpec":          schema_pkg_apis_jaegertracing_v1_JaegerDependenciesSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerEsIndexCleanerSpec":        schema_pkg_apis_jaegertracing_v1_JaegerEsIndexCleanerSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerIngesterSpec":              schema_pkg_apis_jaegertracing_v1_JaegerIngesterSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerIngressOpenShiftSpec":      schema_pkg_apis_jaegertracing_v1_JaegerIngressOpenShiftSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerIngressSpec":               schema_pkg_apis_jaegertracing_v1_JaegerIngressSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerIngressTLSSpec":            schema_pkg_apis_jaegertracing_v1_JaegerIngressTLSSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerQuerySpec":                 schema_pkg_apis_jaegertracing_v1_JaegerQuerySpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerSamplingSpec":              schema_pkg_apis_jaegertracing_v1_JaegerSamplingSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerSpec":                      schema_pkg_apis_jaegertracing_v1_JaegerSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerStatus":                    schema_pkg_apis_jaegertracing_v1_JaegerStatus(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerStorageSpec":               schema_pkg_apis_jaegertracing_v1_JaegerStorageSpec(ref),
-               "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1.JaegerUISpec":                    schema_pkg_apis_jaegertracing_v1_JaegerUISpec(ref),
+               "./pkg/apis/jaegertracing/v1.ElasticsearchSpec":               schema_pkg_apis_jaegertracing_v1_ElasticsearchSpec(ref),
+               "./pkg/apis/jaegertracing/v1.Jaeger":                          schema_pkg_apis_jaegertracing_v1_Jaeger(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerAgentSpec":                 schema_pkg_apis_jaegertracing_v1_JaegerAgentSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerAllInOneSpec":              schema_pkg_apis_jaegertracing_v1_JaegerAllInOneSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerCassandraCreateSchemaSpec": schema_pkg_apis_jaegertracing_v1_JaegerCassandraCreateSchemaSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerCollectorSpec":             schema_pkg_apis_jaegertracing_v1_JaegerCollectorSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerCommonSpec":                schema_pkg_apis_jaegertracing_v1_JaegerCommonSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerDependenciesSpec":          schema_pkg_apis_jaegertracing_v1_JaegerDependenciesSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerEsIndexCleanerSpec":        schema_pkg_apis_jaegertracing_v1_JaegerEsIndexCleanerSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerIngesterSpec":              schema_pkg_apis_jaegertracing_v1_JaegerIngesterSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerIngressOpenShiftSpec":      schema_pkg_apis_jaegertracing_v1_JaegerIngressOpenShiftSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerIngressSpec":               schema_pkg_apis_jaegertracing_v1_JaegerIngressSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerIngressTLSSpec":            schema_pkg_apis_jaegertracing_v1_JaegerIngressTLSSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerQuerySpec":                 schema_pkg_apis_jaegertracing_v1_JaegerQuerySpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerSamplingSpec":              schema_pkg_apis_jaegertracing_v1_JaegerSamplingSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerSpec":                      schema_pkg_apis_jaegertracing_v1_JaegerSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerStatus":                    schema_pkg_apis_jaegertracing_v1_JaegerStatus(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerStorageSpec":               schema_pkg_apis_jaegertracing_v1_JaegerStorageSpec(ref),
+               "./pkg/apis/jaegertracing/v1.JaegerUISpec":                    schema_pkg_apis_jaegertracing_v1_JaegerUISpec(ref),
        }
 }
...

@jpkrohling
Copy link
Contributor

jpkrohling commented Feb 25, 2020

Does the jaeger-operator support GOMODULES or build need to be inside the GOPATH?

It does support GOMODULES, and I do remember seeing some problems with the Operator SDK when GOMODULES is active and GOPATH is set.

@jpkrohling
Copy link
Contributor

@rareddy are you still interested in working on this one?

@rareddy
Copy link
Contributor Author

rareddy commented Mar 9, 2020

@jpkrohling yes, I am. just was not able to recreate the issue locally so that I can fix. I give it another try

@jpkrohling
Copy link
Contributor

@rareddy I can reproduce the same problem that the CI is having by running make ci locally. If you don't get any errors when running make ci, then there's something wrong with your setup. If that's the case, please share the output of:

$ go env
$ go version
$ operator-sdk version

You might also want to run rm $(which openapi-gen) && make install-tools, to ensure you are using a recent version of the openapi-gen utility (the one that generates the files in pkg/apis/jaegertracing/v1/zz_generated.openapi.go)

…ient-gen

Signed-off-by: Ramesh Reddy <rareddy@jboss.org>
@codecov
Copy link

codecov bot commented Mar 10, 2020

Codecov Report

Merging #921 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #921   +/-   ##
=======================================
  Coverage   64.45%   64.45%           
=======================================
  Files          82       82           
  Lines        6535     6535           
=======================================
  Hits         4212     4212           
  Misses       2179     2179           
  Partials      144      144           

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 7d8f068...a250f8c. Read the comment docs.

@rareddy
Copy link
Contributor Author

rareddy commented Mar 10, 2020

@jpkrohling I tried your suggestions still was the same issue. Rather than fighting with it, I rolled back the changes I had for openapi generated file which is a mismatch on my end. Now everything looks fine :)

Copy link
Contributor

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

Looks fantastic, thanks @rareddy!

@jpkrohling jpkrohling merged commit d271a44 into jaegertracing:master Mar 11, 2020
@jpkrohling jpkrohling linked an issue Mar 11, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide client-gen generated code to be used by other Operators
2 participants