Skip to content
This repository was archived by the owner on Dec 3, 2024. It is now read-only.
Closed
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
23 changes: 23 additions & 0 deletions charts/container-object-storage-interface-controller/.helmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Patterns to ignore when building packages.
# This supports shell glob matching, relative path matching, and
# negation (prefixed with !). Only one pattern per line.
.DS_Store
# Common VCS dirs
.git/
.gitignore
.bzr/
.bzrignore
.hg/
.hgignore
.svn/
# Common backup files
*.swp
*.bak
*.tmp
*.orig
*~
# Various IDEs
.project
.idea/
*.tmproj
.vscode/
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
apiVersion: v2
name: container-object-storage-interface-controller
description: A Helm chart for container object storage interface controller
sources:
- http://github.com/kubernetes-sigs/container-object-storage-interface-controller
icon: https://upload.wikimedia.org/wikipedia/commons/6/67/Kubernetes_logo.svg
type: application
version: 0.1.0
appVersion: "0.1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
{{/*
Expand the name of the chart.
*/}}
{{- define "container-object-storage-interface-controller.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
{{- end }}

{{/*
Create a default fully qualified app name.
We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
If release name contains chart name it will be used as a full name.
*/}}
{{- define "container-object-storage-interface-controller.fullname" -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this already starting out with 46 chars because the base name is container-object-storage-interface-controller. I wonder how detailed we all think the name should be.

Can we be as short as cosi-controller?
object(-store?)-interface?
object(-store)?-interface-controller?

Copy link

@jouve jouve Jul 10, 2023

Choose a reason for hiding this comment

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

I suggest that resources are named {{ template "container-object-storage-interface-controller.fullname" . }} (there is one of each kind, so no need to suffix the name.

this is how helm generate a default chart (helm create container-object-storage-interface-controller)

{{- if .Values.fullnameOverride }}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- $name := default .Chart.Name .Values.nameOverride }}
{{- if contains $name .Release.Name }}
{{- .Release.Name | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
{{- end }}
{{- end }}
{{- end }}

{{/*
Create chart name and version as used by the chart label.
*/}}
{{- define "container-object-storage-interface-controller.chart" -}}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
{{- end }}

{{/*
Common labels
*/}}
{{- define "container-object-storage-interface-controller.labels" -}}
helm.sh/chart: {{ include "container-object-storage-interface-controller.chart" . }}
{{ include "container-object-storage-interface-controller.selectorLabels" . }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
{{- end }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- end }}

{{/*
Selector labels
*/}}
{{- define "container-object-storage-interface-controller.selectorLabels" -}}
app.kubernetes.io/name: {{ include "container-object-storage-interface-controller.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
{{- end }}

{{/*
Create the name of the service account to use
*/}}
{{- define "container-object-storage-interface-controller.serviceAccountName" -}}
{{- if .Values.serviceAccount.create }}
{{- default (include "container-object-storage-interface-controller.fullname" .) .Values.serviceAccount.name }}
{{- else }}
{{- default "default" .Values.serviceAccount.name }}
{{- end }}
{{- end }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ include "container-object-storage-interface-controller.fullname" . }}-sa
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/part-of: container-object-storage-interface
{{- include "container-object-storage-interface-controller.labels" . | nindent 4 }}
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ include "container-object-storage-interface-controller.fullname" . }}-objectstorage-controller
Copy link

Choose a reason for hiding this comment

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

as it's the only deployment in the release, I suggest to not add a suffix

labels:
app.kubernetes.io/component: controller
Copy link

Choose a reason for hiding this comment

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

I think component is usefull if there are multiple components in a chart

app.kubernetes.io/part-of: container-object-storage-interface
{{- include "container-object-storage-interface-controller.labels" . | nindent 4 }}
spec:
replicas: {{ .Values.objectstorageController.replicas }}
selector:
matchLabels:
app.kubernetes.io/component: controller
Copy link

Choose a reason for hiding this comment

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

why do you need all these labels in selector.matchLabels ?

app.kubernetes.io/name: container-object-storage-interface-controller
app.kubernetes.io/part-of: container-object-storage-interface
app.kubernetes.io/version: main
{{- include "container-object-storage-interface-controller.selectorLabels" . | nindent 6 }}
template:
metadata:
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/name: container-object-storage-interface-controller
Copy link

Choose a reason for hiding this comment

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

selectorLabels already includes app.kubernetes.io/name

app.kubernetes.io/part-of: container-object-storage-interface
Copy link

Choose a reason for hiding this comment

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

I think "part-of" is used when you deploy a component as part of something else, but container-object-storage-interface is standalone, so I would remove it

app.kubernetes.io/version: main
Copy link

Choose a reason for hiding this comment

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

version should be consistent with AppVersion or image.tag, not hardcoded

{{- include "container-object-storage-interface-controller.selectorLabels" . | nindent 8 }}
spec:
containers:
- args:
- --v=5
env:
- name: KUBERNETES_CLUSTER_DOMAIN
value: {{ quote .Values.kubernetesClusterDomain }}
Copy link

Choose a reason for hiding this comment

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

most charts I know use .Values.clusterDomain

image: {{ .Values.objectstorageController.objectstorageController.image.repository
}}:{{ .Values.objectstorageController.objectstorageController.image.tag | default
.Chart.AppVersion }}
imagePullPolicy: {{ .Values.objectstorageController.objectstorageController.imagePullPolicy
}}
name: objectstorage-controller
resources: {}
Copy link

Choose a reason for hiding this comment

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

resources should be customizable (as it is with the default from helm create)

serviceAccountName: {{ include "container-object-storage-interface-controller.fullname"
. }}-sa
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ include "container-object-storage-interface-controller.fullname" . }}-objectstorage-controller
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/part-of: container-object-storage-interface
{{- include "container-object-storage-interface-controller.labels" . | nindent 4 }}
rules:
- apiGroups:
- coordination.k8s.io
resources:
- leases
verbs:
- get
- watch
- list
- delete
- update
- create
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ include "container-object-storage-interface-controller.fullname" . }}-objectstorage-controller
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/part-of: container-object-storage-interface
{{- include "container-object-storage-interface-controller.labels" . | nindent 4 }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: '{{ include "container-object-storage-interface-controller.fullname" . }}-objectstorage-controller'
subjects:
- kind: ServiceAccount
name: '{{ include "container-object-storage-interface-controller.fullname" . }}-sa'
namespace: '{{ .Release.Namespace }}'
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ include "container-object-storage-interface-controller.fullname" . }}-objectstorage-controller
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/part-of: container-object-storage-interface
{{- include "container-object-storage-interface-controller.labels" . | nindent 4 }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: '{{ include "container-object-storage-interface-controller.fullname" . }}-role'
subjects:
- kind: ServiceAccount
name: '{{ include "container-object-storage-interface-controller.fullname" . }}-sa'
namespace: '{{ .Release.Namespace }}'
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
apiVersion: rbac.authorization.k8s.io/v1
Copy link

Choose a reason for hiding this comment

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

its weird that there is the SA in deployment, ClusterRole here and 3 rbac objects in objectstorage-controller-rbac.yaml

kind: ClusterRole
metadata:
name: {{ include "container-object-storage-interface-controller.fullname" . }}-role
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/part-of: container-object-storage-interface
{{- include "container-object-storage-interface-controller.labels" . | nindent 4 }}
rules:
- apiGroups:
- objectstorage.k8s.io
resources:
- bucketclaims
- bucketaccesses
- bucketclaims/status
- bucketaccesses/status
verbs:
- get
- list
- watch
- update
- apiGroups:
- objectstorage.k8s.io
resources:
- buckets
verbs:
- get
- list
- watch
- update
- create
- delete
- apiGroups:
- objectstorage.k8s.io
resources:
- bucketclasses
- bucketaccessclasses
verbs:
- get
- list
- apiGroups:
- ""
resources:
- events
verbs:
- list
- watch
- create
- update
- patch
- apiGroups:
- ""
resources:
- configmaps
- serviceaccounts
verbs:
- list
- get
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
kubernetesClusterDomain: cluster.local
Copy link

Choose a reason for hiding this comment

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

please keep the values as generated by helm create as it provides documentation

objectstorageController:
objectstorageController:
image:
Copy link

Choose a reason for hiding this comment

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

too many nesting, as there is only 1 image, I suggest image at the top level (as is the default generated by helm create )

repository: gcr.io/k8s-staging-sig-storage/objectstorage-controller
tag: v20221027-v0.1.1-8-g300019f
Copy link

Choose a reason for hiding this comment

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

I you use .Values.image.tag | default .Chart.AppVersion , then it should be empty here

imagePullPolicy: Always
replicas: 1
Copy link

Choose a reason for hiding this comment

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

helm create use replicaCount, I suggest to keep the same name