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

Update jaeger operator to support strimzi operator 0.23.0 #1495

Merged
merged 6 commits into from
Jul 6, 2021

Conversation

rubenvp8510
Copy link
Collaborator

Signed-off-by: Ruben Vargas [email protected]

Which problem is this PR solving?

  • This PR adds compatibility with strimzi operator 0.23

Short description of the changes

  • Migrated kafka CRD to v1beta2 API version

@codecov
Copy link

codecov bot commented Jun 30, 2021

Codecov Report

Merging #1495 (8703208) into master (da66abe) will increase coverage by 0.02%.
The diff coverage is 96.15%.

❗ Current head 8703208 differs from pull request most recent head 685c5f4. Consider uploading reports for the commit 685c5f4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1495      +/-   ##
==========================================
+ Coverage   87.26%   87.28%   +0.02%     
==========================================
  Files          90       90              
  Lines        5025     5033       +8     
==========================================
+ Hits         4385     4393       +8     
  Misses        484      484              
  Partials      156      156              
Impacted Files Coverage Δ
pkg/strategy/strategy.go 95.58% <ø> (ø)
pkg/controller/jaeger/kafka.go 52.56% <66.66%> (ø)
pkg/controller/jaeger/kafkauser.go 52.00% <100.00%> (ø)
pkg/inventory/kafka.go 100.00% <100.00%> (ø)
pkg/inventory/kafkauser.go 100.00% <100.00%> (ø)
pkg/kafka/streaming.go 100.00% <100.00%> (ø)

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 da66abe...685c5f4. Read the comment docs.

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.

I think we talked about expanding the compatibility matrix in the readme to mention which Strimzi versions are supported for each release. This change isn't part of this PR.

@rubenvp8510
Copy link
Collaborator Author

I think we talked about expanding the compatibility matrix in the readme to mention which Strimzi versions are supported for each release. This change isn't part of this PR.

Not yet, but I'll add the matrix to this PR ;)

@rubenvp8510 rubenvp8510 force-pushed the update-strimzi branch 2 times, most recently from 9d75470 to fd1de7f Compare July 2, 2021 03:14
@rubenvp8510
Copy link
Collaborator Author

I just added the matrix to the README.

README.md Outdated
|-----------------|----------------------|
| v1.23 | v1.19+ |
| v1.22 | v1.18 to v1.20 |
| Jaeger Operator | Kubernetes | Strimzi Operator |
Copy link
Contributor

Choose a reason for hiding this comment

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

Reading the doc in a broader context, I believe this table should be moved one section up, as it includes versions from two sections.

@rafilkmp3
Copy link

Please merge this, latest version its failing to deploy since operator droped suport to v1beta2

https://strimzi.io/docs/operators/0.24.0/deploying.html#assembly-upgrade-resources-str

image

@rubenvp8510 rubenvp8510 requested a review from jpkrohling July 6, 2021 02:54
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.

I'll approve as is but will send another PR with a suggestion for the readme.

@jpkrohling jpkrohling enabled auto-merge (squash) July 6, 2021 07:40
@jpkrohling
Copy link
Contributor

Could you please fix the conflicts?

@jpkrohling jpkrohling merged commit 3dd1ef3 into jaegertracing:master Jul 6, 2021
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.

3 participants