uvashishtha opened a new pull request, #1235:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1235
## What is the purpose of the change
Since FLINK-30411, `DefaultValidator#validateTmSpec` and `#validateJmSpec`
check the memory configuration with `processSpecFromConfig` only when the spec
does not set memory through `resources` or the deprecated `resource`. When it
does, the memory options in `flinkConfiguration` are never checked against it.
So an inconsistent memory configuration passes validation, webhook included,
and the operator only finds out on deploy, after it has already shut down the
running cluster. Reproduced on operator 1.16.1 with a session `FlinkDeployment`
that sets `spec.taskManager.resources.requests.memory` (and again with the
deprecated `resource.memory`), updated with `taskmanager.memory.network.max:
32m`, below the default `taskmanager.memory.network.min` of 64m: the update is
admitted, the JobManager deployment is deleted, and the redeploy fails with
```
IllegalConfigurationException: TaskManager memory configuration failed:
Inconsistently configured Key: 'taskmanager.memory.network.fraction' ... min
value must be less or equal to max value
```
leaving the deployment in `MISSING`/`UPGRADING` until the spec is fixed.
When the validation does run, any failure is reported as "TaskManager
resource memory must be defined ...", so the same conflict with the memory set
only in `flinkConfiguration` is reported as missing memory.
## Brief change log
- `ResourceConfigUtils#getProcessMemory` returns the process memory a
JobManager/TaskManager spec sets, with the precedence `FlinkConfigBuilder`
already applied (`resources` once it has any requirements, otherwise the
deprecated `resource`). `FlinkConfigBuilder#applyResourceConfig` sets the
memory from it (first commit, no behavior change).
- `DefaultValidator` sets that memory as `*.memory.process.size` and always
validates the result with `processSpecFromConfig`. It reports "must be defined"
only when no memory is configured at all, and otherwise the exception's message
("TaskManager memory configuration failed: ..."). The resource checks now run
before it, so a memory that does not parse is still reported as a parse error.
- `ResourceConfigUtils#isMemoryDefined` lost its only caller and is removed.
Two specs that were accepted before are now rejected, both of which failed
on deploy:
- a spec whose memory cannot hold Flink's fixed overheads, e.g. a 100 byte
TaskManager (`testValidationWithoutDefaultConfig` used one to check that a
memory without a unit parses; it now uses `"2147483648"`);
- `resources` with no memory (e.g. cpu only) next to a deprecated
`resource.memory`, which `FlinkConfigBuilder` ignores.
## Verifying this change
This change added tests and can be verified as follows:
- `DefaultValidatorTest#testInconsistentMemoryConfigurationIsRejected`:
TaskManager and JobManager, through `resources` and the deprecated `resource`.
-
`DefaultValidatorTest#testInconsistentMemoryConfigurationWithoutSpecMemoryIsRejected`:
memory set only in `flinkConfiguration` is reported as inconsistent, not
missing.
-
`DefaultValidatorTest#testDeprecatedResourceMemoryIsIgnoredWhenResourcesAreSet`.
- `ResourceConfigUtilsTest#testGetProcessMemory`.
- The three `DefaultValidatorTest` cases fail without the fix. `mvn verify
-pl flink-kubernetes-operator` passes (2270 tests).
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changes to the `CustomResourceDescriptors`:
no
- Core observer or reconciler logic that is regularly executed: yes,
`DefaultValidator`, which the controller runs on every reconcile and the
webhook on admission. It now validates the memory configuration of every
deployment; the config it checks starts from the same defaults as the deploy
config, and the spec's process memory is the only memory option
`FlinkConfigBuilder` sets.
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
---
##### Was generative AI tooling used to co-author this PR?
- [x] Yes (Claude Code, Claude Opus 5.5)
Generated-by: Claude Code (Claude Opus 5.5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
