search

stackabletech / druid-operator

An Operator for Apache Druid for Stackable Data Platform
Other
10 stars 0 forks source link

feat: Reduce CRD size #584

Closed sbernauer closed 1 month ago

sbernauer commented 2 months ago

Description

Before 2.4M After operator-rs bump (to pull in https://github.com/stackabletech/operator-rs/pull/821): 289K After extraVolumes schema change: 183K :rocket:

Definition of Done Checklist

# Author
- [x] Changes are OpenShift compatible
- [x] CRD changes approved
- [x] CRD documentation for all fields, following the [style guide](https://docs.stackable.tech/home/nightly/contributor/docs-style-guide).
- [x] Helm chart can be installed and deployed operator works
- [ ] Integration tests passed (for non trivial changes)
- [x] Changes need to be "offline" compatible
# Reviewer
- [ ] Code contains useful comments
- [ ] Code contains useful logging statements
- [ ] (Integration-)Test cases added
- [ ] Documentation added or updated. Follows the [style guide](https://docs.stackable.tech/home/nightly/contributor/docs-style-guide).
- [ ] Changelog updated
- [ ] Cargo.toml only contains references to git tags (not specific commits or branches)
# Acceptance
- [ ] Feature Tracker has been updated
- [ ] Proper release label has been added
- [ ] [Roadmap](https://github.com/orgs/stackabletech/projects/25/views/1) has been updated
NickLarsenNZ commented 2 months ago

I'm curious what the CRD docs would look like. Can we link to somewhere for the rest of the schema? Do we generate two schemas, a full (for docs) and limited (to apply to k8s)?

Although the end-user loses validation directly from k8s-api, the operator will not accept an invalid CR. If we want to shift validation left, we can then look at validating webhooks.

And as you also mentioned, hopefully some day we can bring it back in if Kubernetes handles references to sub-schemas.

NickLarsenNZ commented 2 months ago

Just want to make sure @nightkr sees this, before we continue. Related PR: https://github.com/stackabletech/operator-rs/pull/821

sbernauer commented 2 months ago

Please vote if you agree with this change :+1: / :-1: , I personally don't see any way around this when we want to have CRD versioning

nightkr commented 1 month ago

Ideally I would like to keep it for the latest version even if we prune it for the older..

sbernauer commented 1 month ago

Ideally I would like to keep it for the latest version even if we prune it for the older..

That's an interesting idea, thanks! Lets say we start with v1 (full). Now we bump and have v2 (full) and v1 (without PodTemplate schema). By doing so we would alter the schema of v1 after the fact - does that cause any harm? It sounds dangerous to me, but it might be totally safe. But It might not fit the mental model of the stability of CRD versions :thinking:

sbernauer commented 1 month ago

Given how big the CRD versioning thing already is (months of work already), I think I would prefer to unblock further work by not including the schema for now (we need the size reduction to continue).

Later on we can look into improving the schema, maybe Kubernetes supports sub-schemas at some point or we do something like the suggested "keep it for the latest version even if we prune it for the older".

I hope that works for everyone!

sbernauer commented 1 month ago

Started https://testing.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/druid-operator-it-custom/10/

sbernauer commented 1 month ago

https://testing.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/druid-operator-it-custom/11/