Deal with conflict between Connect and Connect S2I #2463
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Jakub Scholz <www@scholzj.com>
Signed-off-by: Jakub Scholz <www@scholzj.com>
Signed-off-by: Jakub Scholz <www@scholzj.com>
scholzj
force-pushed
the
deal-with-connect-and-connect-s2i-conflict
branch
from
9911eea
to
eece637
Compare
tombentley
reviewed
Jan 28, 2020
| // There is a KafkaConnectS2I with the same name which is older than this KafkaConnect | ||
| && kafkaConnect.getMetadata().getCreationTimestamp().compareTo(otherConnect.getMetadata().getCreationTimestamp()) > 0) { | ||
| return Future.failedFuture("Both KafkaConnect and KafkaConnectS2I exist with the same name. " + | ||
| "KafkaConnectS2I seems to exists longer and will be used while this custom resource will be ignored."); |
There was a problem hiding this comment.
Suggested change
| "KafkaConnectS2I seems to exists longer and will be used while this custom resource will be ignored."); | |
| "KafkaConnectS2I is older and will be used while this custom resource will be ignored."); |
| // There is a KafkaConnect with the same name which is older than or equally old as this KafkaConnectS2I | ||
| && kafkaConnectS2I.getMetadata().getCreationTimestamp().compareTo(otherConnect.getMetadata().getCreationTimestamp()) >= 0) { | ||
| return Future.failedFuture("Both KafkaConnect and KafkaConnectS2I exist with the same name. " + | ||
| "KafkaConnect seems to exists longer and will be used while this custom resource will be ignored."); |
There was a problem hiding this comment.
Suggested change
| "KafkaConnect seems to exists longer and will be used while this custom resource will be ignored."); | |
| "KafkaConnect is older and will be used while this custom resource will be ignored."); |
Signed-off-by: Jakub Scholz <www@scholzj.com>
|
Hehe, good catch. It did work, because the service names are the same, so it actually still connects somewhere and handles the connector. But I fixed the code to make it a bit more clean now. |
tombentley
approved these changes
Jan 28, 2020
|
@scholzj I think you might need to make a small change to the docs, which IIRC mentions the old " |
|
@tombentley Any idea where in the docs? I didn't found anything. |
|
@scholzj I took a look and I don't see it either. Maybe I misremembered. |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change
Description
When both Kafka Connect and Kafka Connect S2I exist in the same namespace with the same name they conflict with each other. They share (and overwrite) each others services, config maps etc. This PR adds checks and only the older one from the two will be reconciled. The older or newer one is decided based on the Creation timestamp in metadata.
This PR doesn't really deal with situation where such two Connect and Connect S2I instances already exist - it will error the status and stop reconciling one of them, but not delete the pods etc. But given how disfunctional they would be I guess nobody is using this. When the user deletes one of the CRs thi should be resolved through the garbage collection.
Checklist