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

Added upgrade mechanism for managed Jaeger instances #476

Merged
merged 1 commit into from
Jul 30, 2019

Conversation

jpkrohling
Copy link
Contributor

@jpkrohling jpkrohling commented Jun 21, 2019

This PR is still in draft mode. The purpose for now is to agree on the main assumptions, like versioning and how to apply the upgrades.

Closes #42

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

@jpkrohling jpkrohling requested review from pavolloffay and objectiser and removed request for pavolloffay June 21, 2019 14:57
@codecov
Copy link

codecov bot commented Jun 21, 2019

Codecov Report

Merging #476 into master will decrease coverage by 0.16%.
The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
- Coverage    91.7%   91.53%   -0.17%     
==========================================
  Files          64       65       +1     
  Lines        3181     3189       +8     
==========================================
+ Hits         2917     2919       +2     
- Misses        184      188       +4     
- Partials       80       82       +2
Impacted Files Coverage Δ
pkg/apis/jaegertracing/v1/jaeger_types.go 100% <ø> (ø) ⬆️
pkg/controller/jaeger/jaeger_controller.go 38.14% <0%> (-2.07%) ⬇️
pkg/deployment/all-in-one.go 100% <100%> (ø) ⬆️
pkg/deployment/collector.go 100% <100%> (ø) ⬆️
pkg/deployment/ingester.go 100% <100%> (ø) ⬆️
pkg/deployment/agent.go 100% <100%> (ø) ⬆️
pkg/deployment/query.go 100% <100%> (ø) ⬆️
pkg/storage/cassandra_dependencies.go 100% <100%> (ø) ⬆️
pkg/controller/jaeger/upgrade.go 50% <50%> (ø)

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 35880d0...5cc91ab. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 21, 2019

Codecov Report

Merging #476 into master will decrease coverage by 0.42%.
The diff coverage is 64.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
- Coverage   91.98%   91.56%   -0.43%     
==========================================
  Files          69       73       +4     
  Lines        3557     3615      +58     
==========================================
+ Hits         3272     3310      +38     
- Misses        199      213      +14     
- Partials       86       92       +6
Impacted Files Coverage Δ
pkg/deployment/args.go 100% <ø> (ø) ⬆️
pkg/apis/jaegertracing/v1/jaeger_types.go 100% <ø> (ø) ⬆️
pkg/upgrade/versions.go 100% <100%> (ø)
pkg/deployment/all-in-one.go 100% <100%> (ø) ⬆️
pkg/deployment/collector.go 100% <100%> (ø) ⬆️
pkg/deployment/ingester.go 100% <100%> (ø) ⬆️
pkg/deployment/agent.go 100% <100%> (ø) ⬆️
pkg/deployment/query.go 100% <100%> (ø) ⬆️
pkg/upgrade/v1_13_1.go 100% <100%> (ø)
pkg/storage/cassandra_dependencies.go 100% <100%> (ø) ⬆️
... and 8 more

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 584224f...f851e08. Read the comment docs.

@objectiser
Copy link
Contributor

@jpkrohling Wanted to check one thing - currently it appears as if the jaeger version can be updated while the operator is running, as the controller checks for upgrading the version when a change is applied? Thought this would only be done when the operator is first started?

How would the version be updated for a running instance?

@jpkrohling
Copy link
Contributor Author

jpkrohling commented Jun 24, 2019

The usual case will be that the operator will get all the existing Jaeger instances and upgrade them, but there might be some concurrency-related situations (or permission-related cases) where the operator couldn't see an existing Jaeger instance when it first started. The reconcile logic will catch these cases and upgrade the individual instances on demand.

How would the version be updated for a running instance?

Pretty much all Jaeger instances will be running when the upgrade process starts. Each upgrade can define how the upgrade will work. In some cases, we might need to stop the storage, apply a new schema or data migration, and start the collector again. In other cases, we can just change the deployment to use a newer image and Kubernetes will take care of the rolling update.

@jpkrohling
Copy link
Contributor Author

@objectiser : have you had a chance to look into this PR? Do you think this would be appropriate for our planned use cases?

@objectiser
Copy link
Contributor

@jpkrohling Yes I've added a number of comments that you have responded to :) Yes looks a long the right lines for what we need. Not sure should be added for 1.13.x yet though - possibly plan for 1.14?

@jpkrohling
Copy link
Contributor Author

The coverage is lacking because it's not easy to simulate client problems with the fake client.

pkg/cmd/start/main.go Outdated Show resolved Hide resolved
pkg/storage/cassandra_dependencies.go Outdated Show resolved Hide resolved
pkg/upgrade/upgrade.go Show resolved Hide resolved
pkg/upgrade/v1_11_0.go Outdated Show resolved Hide resolved
pkg/upgrade/versions.go Outdated Show resolved Hide resolved
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@jpkrohling jpkrohling merged commit 60f5ed6 into jaegertracing:master Jul 30, 2019
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.

Support update of the underlying Jaeger version
3 participants