Repository navigation
Let a deployment state its own probes, and fix CRD generation - #13
Merged
Merged
Conversation
The operator hardcodes timeoutSeconds: 1 on all three probes and the CRD
exposed no probe fields at all, so an instance that answers /health slowly is
killed by the kubelet 30 seconds later. Under CPU contention Seq's own log
reports a /api response taking 12.7 seconds; seq-server-0 carries 346 restarts
on one cluster and 282 on another, every one of them exit 143 rather than a
fault in Seq.
spec.deployment now carries livenessProbe, readinessProbe and startupProbe. A
probe given there replaces the operator's outright rather than merging into it:
merging would let a caller set one field, inherit the rest, and read back a
probe that is neither what they wrote nor what was chosen for them, and it
offers no way to clear a field the operator sets. Left unset, the emitted
probes are exactly what they are today.
None of that could reach the CRD, because the pinned kubeops.cli
9.1.6-alethic.5 resolves from no configured feed, and KubeOps.Operator.targets
probes the CLI with IgnoreExitCode and invokes generation with ContinueOnError
-- so generation was skipped and the build still reported success. The tool is
pinned to the stock 9.11.8, matching the KubeOps.Operator and KubeOps.Generator
references the project already carries.
Regenerating on stock leaves four of the five CRDs byte-identical apart from
nullable annotations, and every enum identical -- the fork's patch is
JsonStringEnumMemberNameAttribute handling, which nothing here declares. What
it does change is ResourceQuantity, which 9.1.x transpiled as an object of
{format, value}. Kubernetes quantities are strings, so spec.deployment.resources
could not be given an ordinary value like 250m, which is why Seq has run
BestEffort; env[].valueFrom.resourceFieldRef.divisor carried the same fault.
Namespaces are declared through EntityRbac. An Instance's permissions are
evaluated against the namespace object itself, but nothing inferred that from
the controller's entity types, so the rule was patched in by hand wherever the
generated role was copied. The chart's ClusterRole is now a straight copy of
the generated role with nothing added to it.
This file contains hidden or 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
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.
What this changes
Probes are settable.
spec.deploymentgainslivenessProbe,readinessProbeandstartupProbe. The operator hardcodestimeoutSeconds: 1on all three and the CRD exposed no probe fields, so an instance that answers/healthslowly is killed 30s later —seq-server-0carries 346 restarts on one cluster and 282 on another, all exit 143.A probe given on the deployment replaces the operator's outright rather than merging. Merging would let a caller set one field, inherit the rest, and read back a probe that is neither what they wrote nor what was chosen for them — and gives no way to clear a field the operator sets. Unset keeps today's probes byte for byte.
CRD generation works again. The pinned
kubeops.cli 9.1.6-alethic.5resolves from no configured feed, andKubeOps.Operator.targetsprobes the CLI withIgnoreExitCodethen invokes generation withContinueOnError— so generation was skipped and the build still reported success. Now pinned to stock 9.11.8, matching theKubeOps.Operator/KubeOps.Generatorreferences already in the project.spec.deployment.resourcesbecomes usable. 9.1.x transpiledResourceQuantityas an object{format, value}; Kubernetes quantities are strings, socpu: 250mnever matched the schema. That is why Seq has been running BestEffort.env[].valueFrom.resourceFieldRef.divisorhad the same fault. Stock 9.11.8 emitstype: string.Namespaces are declared via
EntityRbac. An Instance's permissions are evaluated against the namespace object, but nothing inferred that, so the rule was hand-patched wherever the generated role was copied. The chart's ClusterRole is now a straight copy of the generated role.On dropping the fork
Regenerating on stock leaves four of the five CRDs byte-identical apart from
nullableannotations, and every enum identical. The fork's transpiler patch isJsonStringEnumMemberNameAttributehandling, which nothing in this repo declares — so it costs this operator nothing to drop, and the fork's v9.1.5 base is what carried theResourceQuantitybug.Worth noting: the fork is still based on v9.1.5, so any other operator generated with it has the same
resourcesfault.Reviewer note — one grant you may want to reject
Adding
[EntityRbac(typeof(V1Namespace), ...)]withGet | List | Watchalso makes the generator emit anamespaces/statusrule withget,update,patch. The operator never writes namespace status, so that is privilege it does not need. The alternative is to drop the attribute and keep patching thenamespacesrule in by hand on every regeneration. Happy to revert if you would rather not carry the extra grant.Follow-up, not in this PR
This needs a release before it does anything:
sweep-appdev-envpins the operator at0.1.0-pre.132in the seq-operatorOCIRepository, andcrds: CreateReplacewill replace the CRD when a new chart version lands. The matchingspec.deployment.resourceschange for the Seq instance is held back until then — against the currently deployed CRD it will not validate.