Skip to content
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

Upgrade flux-operator to 0.9.0 #362

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ annotations:
- name: Upstream Project
url: https://github.com/controlplaneio-fluxcd/flux-operator
apiVersion: v2
appVersion: v0.6.0
appVersion: v0.9.0
description: 'A Helm chart for deploying the Flux Operator. '
home: https://github.com/controlplaneio-fluxcd
icon: https://raw.githubusercontent.com/cncf/artwork/main/projects/flux/icon/color/flux-icon-color.png
Expand All @@ -18,13 +18,11 @@ keywords:
- gitops
kubeVersion: '>=1.22.0-0'
maintainers:
- email: [email protected]
name: Stefan Prodan
- name: Soule Ba
url: [email protected]
- email: [email protected]
name: ControlPlane Flux Team
name: flux-operator
sources:
- https://github.com/controlplaneio-fluxcd/flux-operator
- https://github.com/controlplaneio-fluxcd/charts
type: application
version: 0.6.0
version: 0.9.0
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# flux-operator

![Version: 0.6.0](https://img.shields.io/badge/Version-0.6.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v0.6.0](https://img.shields.io/badge/AppVersion-v0.6.0-informational?style=flat-square)
![Version: 0.9.0](https://img.shields.io/badge/Version-0.9.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v0.9.0](https://img.shields.io/badge/AppVersion-v0.9.0-informational?style=flat-square)

The [Flux Operator](https://github.com/controlplaneio-fluxcd) provides a declarative API
for the installation and upgrade of CNCF [Flux](https://fluxcd.io) and the
The [Flux Operator](https://github.com/controlplaneio-fluxcd/flux-operator) provides a
declarative API for the installation and upgrade of CNCF [Flux](https://fluxcd.io) and the
ControlPlane [enterprise distribution](https://control-plane.io/enterprise-for-flux-cd/).

The operator automates the patching for hotfixes and CVEs affecting the Flux controllers container images
Expand Down Expand Up @@ -49,6 +49,7 @@ see the Flux Operator [documentation](https://fluxcd.control-plane.io/operator/)
| resources | object | `{"limits":{"cpu":"1000m","memory":"1Gi"},"requests":{"cpu":"100m","memory":"64Mi"}}` | Container resources requests and limits settings. |
| securityContext | object | `{"allowPrivilegeEscalation":false,"capabilities":{"drop":["ALL"]},"readOnlyRootFilesystem":true,"runAsNonRoot":true,"seccompProfile":{"type":"RuntimeDefault"}}` | Container security context settings. The default is compliant with the pod security restricted profile. |
| serviceAccount | object | `{"automount":true,"create":true,"name":""}` | Pod service account settings. The name of the service account defaults to the release name. |
| serviceMonitor | object | `{"create":false,"interval":"60s","labels":{},"scrapeTimeout":"30s"}` | Prometheus Operator scraping settings. |
| tolerations | list | `[]` | Pod tolerations settings. |

## Source Code
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.15.0
controller-gen.kubebuilder.io/version: v0.16.1
helm.sh/resource-policy: keep
labels:
app.kubernetes.io/instance: '{{ .Release.Name }}'
Expand Down Expand Up @@ -77,6 +77,12 @@ spec:
NetworkPolicy restricts network access to the current namespace.
Defaults to true.
type: boolean
tenantDefaultServiceAccount:
description: |-
TenantDefaultServiceAccount is the name of the service account
to use as default when the multitenant lockdown is enabled.
Defaults to the 'default' service account from the tenant namespace.
type: string
type:
default: kubernetes
description: |-
Expand Down Expand Up @@ -202,6 +208,29 @@ spec:
type: object
type: array
type: object
migrateResources:
default: true
description: |-
MigrateResources instructs the controller to migrate the Flux custom resources
from the previous version to the latest API version specified in the CRD.
Defaults to true.
type: boolean
sharding:
description: Sharding holds the specification of the sharding configuration.
properties:
key:
default: sharding.fluxcd.io/key
description: Key is the label key used to shard the resources.
type: string
shards:
description: Shards is the list of shard names.
items:
type: string
minItems: 1
type: array
required:
- shards
type: object
Comment on lines +218 to +233
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Potential Missing Tests for Sharding Feature

It appears that there are no existing tests specifically targeting the new sharding field in crds.yaml. To ensure the reliability and correctness of the sharding configuration, it's recommended to implement comprehensive tests covering various shard setups and edge cases.

  • File under review: packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml (lines 218-233)
🔗 Analysis chain

LGTM. Ensure thorough testing of the new sharding feature.

The addition of the sharding field introduces a powerful sharding capability to the Flux operator. The key field allows customization of the label key used for sharding, while the required shards field ensures a complete sharding configuration.

This feature could significantly impact how resources are distributed and managed. Please ensure thorough testing of the sharding feature, including edge cases and various shard configurations. Consider adding integration tests specifically for this feature. Run the following command to check for any existing tests related to sharding:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for existing tests related to sharding

# Test: Search for test files or test cases related to sharding
rg -i "test|spec" | rg -i "shard|partition"

Length of output: 33661

storage:
description: |-
Storage holds the specification of the source-controller
Expand Down Expand Up @@ -274,7 +303,6 @@ spec:
type: boolean
required:
- distribution
- wait
type: object
status:
description: FluxInstanceStatus defines the observed state of FluxInstance
Expand Down Expand Up @@ -307,16 +335,8 @@ spec:
conditions:
description: Conditions contains the readiness conditions of the object.
items:
description: "Condition contains details for one aspect of the current
state of this API Resource.\n---\nThis struct is intended for
direct use as an array at the field path .status.conditions. For
example,\n\n\n\ttype FooStatus struct{\n\t // Represents the
observations of a foo's current state.\n\t // Known .status.conditions.type
are: \"Available\", \"Progressing\", and \"Degraded\"\n\t //
+patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t
\ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\"
patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t
\ // other fields\n\t}"
description: Condition contains details for one aspect of the current
state of this API Resource.
properties:
lastTransitionTime:
description: |-
Expand Down Expand Up @@ -357,12 +377,7 @@ spec:
- Unknown
type: string
type:
description: |-
type of condition in CamelCase or in foo.example.com/CamelCase.
---
Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
useful (see .node.status.conditions), the ability to deconflict is important.
The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
description: type of condition in CamelCase or in foo.example.com/CamelCase.
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
Expand Down Expand Up @@ -429,7 +444,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.15.0
controller-gen.kubebuilder.io/version: v0.16.1
helm.sh/resource-policy: keep
labels:
app.kubernetes.io/instance: '{{ .Release.Name }}'
Expand Down Expand Up @@ -622,16 +637,8 @@ spec:
conditions:
description: Conditions contains the readiness conditions of the object.
items:
description: "Condition contains details for one aspect of the current
state of this API Resource.\n---\nThis struct is intended for
direct use as an array at the field path .status.conditions. For
example,\n\n\n\ttype FooStatus struct{\n\t // Represents the
observations of a foo's current state.\n\t // Known .status.conditions.type
are: \"Available\", \"Progressing\", and \"Degraded\"\n\t //
+patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t
\ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\"
patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t
\ // other fields\n\t}"
description: Condition contains details for one aspect of the current
state of this API Resource.
properties:
lastTransitionTime:
description: |-
Expand Down Expand Up @@ -672,12 +679,7 @@ spec:
- Unknown
type: string
type:
description: |-
type of condition in CamelCase or in foo.example.com/CamelCase.
---
Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
useful (see .node.status.conditions), the ability to deconflict is important.
The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
description: type of condition in CamelCase or in foo.example.com/CamelCase.
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@ spec:
{{- include "flux-operator.selectorLabels" . | nindent 6 }}
template:
metadata:
{{- with .Values.commonAnnotations }}
annotations:
prometheus.io/scrape: "true"
prometheus.io/port: "8080"
prometheus.io/path: "/metrics"
{{- with .Values.commonAnnotations }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- end }}
labels:
{{- include "flux-operator.labels" . | nindent 8 }}
{{- with .Values.commonLabels }}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{{- if .Values.serviceMonitor.create }}
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
name: {{ include "flux-operator.fullname" . }}
namespace: {{ .Release.Namespace }}
labels:
{{- include "flux-operator.labels" . | nindent 4 }}
{{- with .Values.commonLabels }}
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with .Values.serviceMonitor.labels }}
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with .Values.commonAnnotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
spec:
namespaceSelector:
matchNames:
- {{ .Release.Namespace | quote }}
selector:
matchLabels:
{{- include "flux-operator.selectorLabels" . | nindent 6 }}
endpoints:
- targetPort: 8080
path: /metrics
interval: {{ .Values.serviceMonitor.interval }}
scrapeTimeout: {{ .Values.serviceMonitor.scrapeTimeout }}
Comment on lines +19 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

LGTM: Well-configured ServiceMonitor specification with a minor suggestion.

The ServiceMonitor specification is correctly structured and uses appropriate templating for flexibility and consistency. The endpoints configuration allows for customization of scraping parameters, which is excellent.

Consider making the metrics path configurable by using a Helm value instead of hardcoding it. This would provide more flexibility for users who might want to change the metrics endpoint. For example:

path: {{ .Values.serviceMonitor.metricsPath | default "/metrics" }}

Don't forget to update the values.yaml file to include this new option if you implement this suggestion.

{{- end }}
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,29 @@
},
"type": "object"
},
"serviceMonitor": {
"default": {
"create": false,
"interval": "60s",
"scrapeTimeout": "30s"
},
"properties": {
"create": {
"type": "boolean"
},
"interval": {
"type": "string"
},
"labels": {
"properties": {},
"type": "object"
},
"scrapeTimeout": {
"type": "string"
}
},
"type": "object"
},
"tolerations": {
"items": {
"type": "object"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,21 @@ affinity: # @schema default: {"nodeAffinity":{"requiredDuringSchedulingIgnoredDu
# -- Pod tolerations settings.
tolerations: [ ] # @schema item: object ; uniqueItems: true

# -- Marketplace settings.
marketplace:
type: ""
license: ""
account: ""

# -- If `true`, the container ports (`8080` and `8081`) are exposed on the host network.
hostNetwork: false # @schema default: false

# -- Container extra environment variables.
extraEnvs: [ ] # @schema item: object ; uniqueItems: true

# -- Prometheus Operator scraping settings.
serviceMonitor: # @schema default: {"create":false,"interval":"60s","scrapeTimeout":"30s"}
create: false
interval: 60s
scrapeTimeout: 30s
labels: { }

# -- Marketplace settings.
marketplace:
type: ""
license: ""
account: ""