Addon upgrade flux 2.4#766
Conversation
bugbuilder
requested review from
FogDong,
anoop2811,
charlie0129,
dhiguero and
wangyikewxgm
as code owners
wonderflow
left a comment
wonderflow
left a comment
There was a problem hiding this comment.
appreciate the contribution, please help sign the DCO
Signed-off-by: bugbuilder <nelson@v7n.cl>
Signed-off-by: bugbuilder <nelson@v7n.cl>
Signed-off-by: bugbuilder <nelson@v7n.cl>
Signed-off-by: bugbuilder <nelson@v7n.cl>
|
oops, sorry the conflict happen when I merged other PR |
Signed-off-by: bugbuilder <nelson@v7n.cl>
No worries, already fixed it |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
| memory: "1Gi" | ||
| } | ||
| requests: { | ||
| cpu: "100m" |
There was a problem hiding this comment.
CPU resource request (100m) is significantly lower than the limit (1000m) with a 10:1 ratio, which could lead to resource overcommitment and potential container throttling
| } | ||
| requests: { | ||
| cpu: "100m" | ||
| memory: "64Mi" |
There was a problem hiding this comment.
Memory resource request (64Mi) is significantly lower than the limit (1Gi) with a 16:1 ratio, which could lead to pod evictions if actual usage exceeds requests
| value: _targetNamespace | ||
| }, | ||
| { | ||
| name: "GOMAXPROCS" |
There was a problem hiding this comment.
GOMAXPROCS set to limits.cpu might cause performance issues if container is throttled
| properties: { | ||
| if parameter.imageAutomationControllerOptions != _|_ { | ||
| args: controllerArgs + parameter.imageAutomationControllerOptions | ||
| args: eventAddrArgs + parameter.imageAutomationControllerOptions |
There was a problem hiding this comment.
The code replaces controllerArgs with eventAddrArgs but doesn't properly handle case when notification-controller (which eventAddrArgs depends on) is not deployed
| value: _targetNamespace | ||
| }, | ||
| { | ||
| name: "GOMAXPROCS" |
There was a problem hiding this comment.
Missing validation for CPU limits when setting GOMAXPROCS
| @@ -1,5 +1,5 @@ | |||
| name: fluxcd | |||
| version: 3.0.1 | |||
| version: 3.1.0 | |||
There was a problem hiding this comment.
The version bump is correct, but there's a bug in the newly added notification-controller.cue file
| if parameter.notificationControllerResourceLimits.memory != _|_ { | ||
| memory: parameter.notificationControllerResourceLimits.memory | ||
| } | ||
| if parameter.sourceControllerResourceLimits.memory == _|_ { |
There was a problem hiding this comment.
Incorrect variable reference: using sourceControllerResourceLimits when checking notification controller memory limits
| type: "object" | ||
| } | ||
| served: true | ||
| storage: true |
There was a problem hiding this comment.
Setting v1 as the storage version while keeping older versions served could lead to data loss if not handled properly during upgrade
| type: "date" | ||
| }] | ||
| name: "v1beta1" | ||
| deprecated: true |
There was a problem hiding this comment.
Deprecating v1beta1 and v1beta2 without a migration path might lead to breaking changes for existing users
| type: "string" | ||
| } | ||
| required: ["name"] | ||
| type: "object" |
There was a problem hiding this comment.
The schema for v1 introduces several new fields without specifying defaults that might cause issues for existing resources
|
Still Have #785 |
4 participants
This update includes all the necessary changes to upgrade Flux to its 2.4 GA version, ensuring compatibility and enhanced functionality for the addon. The key changes are:
Some practices, such as priorityClassName, securityContext, and terminationGracePeriodSeconds, were not implemented as they are currently not supported by the webservice component type. Discussions regarding their support are still ongoing in the community, as referenced in kubevela/kubevela#5580.
I would appreciate clarification on what types of tests should be run to ensure the changes meet the project's merge requirements. This would help validate the implementation further and ensure compatibility across different environments.