-
Couldn't load subscription status.
- Fork 0
Fix/soft deleting schema subjects #6
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
Fix/soft deleting schema subjects #6
Conversation
Previous implementation was using DELETE /config/{subject} to remove
schema from registry. According to
https://docs.confluent.io/platform/current/schema-registry/develop/api.html#config,
this endpoint "Deletes the specified subject-level compatibility level
config and reverts to the global default" instead of actually deleting
the schema.
I've decided to only soft-delete the schema (instead of permanent
hard-delete), because the same schema object could potentially be
referenced by other subjects.
1. fixed indentation of serviceAccountName 2. fixed values for SCHEMA_REGISTRY_KEY and SCHEMA_REGISTRY_SECRET
|
@Tomek-Adamczewski First of all thanks for the contribution. I will test your changes soon and get back here |
|
I created a new PR with respect to your changes and an updated parameter which I suggested above, please review here I will close this PR just so as not to keep duplications. One more time, thanks for your contribution |
|
Hi Anton,
Sorry I didn’t get back to you before - I was on PTO and didn’t care much about my laptop ;).
First of all, thanks for creating and maintaining this operator. That’s awesome! Finally something simple that allows structured management of Kafka schemas. I have a feeling that people don’t really understand how important it is, and you’ve created a tool that all Kafka+K8S users should eventually consider in their setup
I’ve seen your PR and I was actually thinking about it, but I was having some concerns:
1. you can’t hard-delete schema without prior soft-delete. So, if you want your flag to work, you should always soft-delete subject (DELETE without permanent param) and then eventually send another DELETE with permanent=true
2. I’m not sure how SR will behave on hard-deletes if the same schema is referenced by another subject. SR data model consists of subjects + versions (one subject has a list of versions), and schemas. Subject version doesn’t “contain” schema, it just references a schema by its ID. If you create a new subject/version but use a schema that was already used by another subject/version, SR will detect that and use the existing schema ID in your version ref. It won’t store duplicates. This means that a single schema can be referenced by multiple subjects/versions. I didn’t test what would happen if you try to delete a schema that’s referenced by another subject, but it’ll definitely fail “somehow”: either SR will block the DELETE and return subject versions that reference the schema you try to delete, or - worst case - it will allow you to delete the schema and the other subject will be broken. In any case, it’ll be difficult to handle such edge-case from the perspective of custom resource (defining only a single subject and its associated schema)
Additionally, my team started using the operator and they identified 2 additional issues:
First one is relatively simple. Since the operator is cluster-wide, can we define schema registry host/port/key/secret per KafkaSchema resource? We have multiple environments as k8s namespaces on the same cluster, and they use separate Kafka brokers + Schema Registries. We would like to have a single operator that deploys “dev” schemas on “dev” SR instance, and “prod” schemas on “prod” instance. This change is backward compatible: let’s assume SR configuration is optional on resource level and the “global” envs are used as defaults/fallback in case you didn’t define those values in your resource
The other one is really tricky. We’re facing synchronization issues between Helm-managed resources that the operator is using: configmap with schema and KafkaSchema resource that references it. In our case, those 2 resources are part of the same chart, as we want to enforce the autonomy of microservices and let publishers define/own schemas for their events. Now, there’s a race condition between those two resources:
1. if you want to delete a schema but k8s already deleted configmap, operator will fail
2. if you modify the schema payload (content of configmap) but the operator is triggered before the configmap was updated in K8S, operator will succeed but the actual schema inside the registry won’t change.
We couldn’t figure out a proper configuration to force proper order/synchronization of the two resources:
1. we could configure a finalizer on configmap to prevent k8s from deleting it, but operator would have to remove that finalizer from the configmap after it cleans up registry
2. we could play with ownerReferences and instruct k8s to delete KafkaSchema before the configmap (and the operator would actually delete the configmap along the way). But that’s tricky since we store both resources in the same Helm chart, and Helm doesn’t have proper support for owner references - we would have to hack it somehow if we don’t want to end up with multiple charts for a single deployment/service
What I came up with so far is this: incubly-oss/kafka-schema-operator#1 - if we inline the schema payload directly in KafkaSchema resource, there are no synchronization issues and things are much simpler. But that’s a breaking change compared to your approach, so I didn’t even propose it as a PR to your repository without discussions :). Do you think it would make sense to change the approach in your model or eventually support both models (KafkaSchema would either contain schema, or configRef - but in this case we should still think how to address the potential synchronization issues that I’ve mentioned above and also in the PR comment).
Regards,
Tomek
On 10 Mar 2024, at 17:10, Anton Poplavnoi ***@***.***> wrote:
I created a new PR with respect to your changes and an updated parameter which I suggested above, please review here<#7>
and feel free to update it with any comments.
I will close this PR just so as not to keep duplications.
One more time, thanks for your contribution
—
Reply to this email directly, view it on GitHub<#6 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AZSHPKD433GA2JGEQFLADL3YXSAYFAVCNFSM6AAAAABEIZZPTSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBXGI4DCNBYGY>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
No description provided.