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

BrokerPodSpec and initContainers resources PLACEHOLDER #645

Closed
Tetragramato opened this issue Aug 9, 2023 · 3 comments · Fixed by #649
Closed

BrokerPodSpec and initContainers resources PLACEHOLDER #645

Tetragramato opened this issue Aug 9, 2023 · 3 comments · Fixed by #649
Assignees
Labels
bug Something isn't working

Comments

@Tetragramato
Copy link

Describe the bug
Trying to put initContainers in a brokerPodSpec, i found that the akri-controller fall in error about the spec resources and PLACEHOLDER

Why i'm trying to do that ? I want an external docker image to act as a broker, BTW to infer akri custom env vars in the configuration file i need to write it with proper values. So i'm trying with an init container to write those values, efore my main broker start (IDK if it make sense ^^)

Output of kubectl get pods,akrii,akric -o wide

NAME                                              READY   STATUS             RESTARTS        AGE     IP            NODE     NOMINATED NODE   READINESS GATES
pod/akri-agent-daemonset-c8dmd                    1/1     Running            3 (7h54m ago)   2d1h    10.42.0.148   khadas   <none>           <none>
pod/akri-udev-discovery-daemonset-nt972           1/1     Running            3 (7h54m ago)   2d1h    10.42.0.152   khadas   <none>           <none>
pod/akri-controller-deployment-64b48bb59d-blbx8   0/1     CrashLoopBackOff   5 (2m42s ago)   6m33s   10.42.0.176   khadas   <none>           <none>

NAME                             CONFIG   SHARED   NODES        AGE
instance.akri.sh/zigbee-abd677   zigbee   false    ["khadas"]   12m

NAME                           CAPACITY   AGE
configuration.akri.sh/zigbee   1          7h13m

Kubernetes Version: [e.g. Native Kubernetes 1.19, MicroK8s 1.19, Minikube 1.19, K3s]
K3S

To Reproduce
1 Put an initContainer in a brokerPodSpec
2 place resources with PLACEHOLDER to this container
3 Run the configuration

My configuration (it's a POC, so it's a draft) :

{{- if .Values.zigbee.configuration.enabled }}
apiVersion: {{ printf "%s/%s" .Values.akri.crds.group .Values.akri.crds.version }}
kind: Configuration
metadata:
  name: zigbee
spec:
  discoveryHandler:
    name: udev
    discoveryDetails: |+
      groupRecursive: true
      udevRules:
      {{- required "Please set at least one udev rule to specify what you want discovered." .Values.zigbee.configuration.discoveryDetails.udevRules | toYaml | nindent 6 }}
  brokerSpec:
    brokerPodSpec:
      initContainers:
      - name: zigbee-init
        image: tetragramato/kubectl-jq:0.1.0
        command: ["cp -f /tmp/configuration.yaml /workdir/configuration.yaml"]
        imagePullPolicy: Always
        volumeMounts:
          - name: config
            mountPath: /tmp/configuration.yaml
            subPath: configuration.yaml
          - name: workdir
            mountPath: /workdir
        resources:
          requests:
            {{`"{{PLACEHOLDER}}"`}} : "1"
          limits:
            {{`"{{PLACEHOLDER}}"`}} : "1"
      containers:
      - name: zigbee-broker
        image: {{ printf "koenkk/zigbee2mqtt:%s" .Values.zigbee.configuration.brokerPod.image.tag | quote }}
        {{- with .Values.zigbee.configuration.brokerPod.image.pullPolicy }}
        imagePullPolicy: {{ . }}
        {{- end }}
        env:
          - name: TZ
            value: "Europe/Paris"
        {{- if .Values.zigbee.configuration.brokerPod.envFrom }}
        envFrom:
        {{- range $val := .Values.zigbee.configuration.brokerPod.envFrom.secretRef }}
        - secretRef:
            name: {{ $val | quote }}
        {{- end }}
        {{- range $val := .Values.zigbee.configuration.brokerPod.envFrom.configMapRef }}
        - configMapRef:
            name: {{ $val | quote }}
        {{- end }}
        {{- end }}
        securityContext:
          privileged: true
        ports:
        - containerPort: 8080
          name: http
          protocol: TCP
        resources:
          requests:
            memory: {{ .Values.zigbee.configuration.brokerPod.resources.memoryRequest }}
            cpu: {{ .Values.zigbee.configuration.brokerPod.resources.cpuRequest }}
          limits:
            memory: {{ .Values.zigbee.configuration.brokerPod.resources.memoryLimit }}
        volumeMounts:
        - name: workdir
          mountPath: /app/data
      volumes:
      - name: config
        configMap:
          name: zigbee2mqtt-config
      - name: workdir
        emptyDir: {}
      {{- with .Values.imagePullSecrets }}
      imagePullSecrets:
        {{- toYaml . | nindent 6 }}
      {{- end }}
    {{- end }}
  instanceServiceSpec:
    type: ClusterIP
    ports:
    - name: http
      port: 8080
      protocol: TCP
      targetPort: http
  {{- if .Values.zigbee.configuration.brokerProperties }}
  brokerProperties:
  {{- range $key, $val := .Values.zigbee.configuration.brokerProperties }}
  {{- $key | nindent 4 }}: {{ $val | quote }}
  {{- end }}
  {{- else }}
  brokerProperties: {}
  {{- end }}
  capacity: 1

Expected behavior
Placing PLACEHOLDER in spec resources, no matter if it's a container or initContainer, i want them to be resolved correctly

Logs (please share snips of applicable logs)

[2023-08-09T14:19:02Z TRACE akri_shared::k8s::pod] create_pod enter
[2023-08-09T14:19:02Z INFO  akri_shared::k8s::pod] create_pod pods.create(...).await?:
[2023-08-09T14:19:03Z ERROR akri_shared::k8s::pod] create_pod pods.create [Ok("{\"apiVersion\":\"v1\",\"kind\":\"Pod\",\"metadata\":{\"labels\":{\"akri.sh/configuration\":\"zigbee\",\"akri.sh/instance\":\"zigbee-abd677\",\"akri.sh/target-node\":\"khadas\",\"app\":\"zigbee-abd677-pod\",\"controller\":\"akri.sh\"},\"name\":\"zigbee-abd677-pod\",\"namespace\":\"stemz-devices\",\"ownerReferences\":[{\"apiVersion\":\"akri.sh/v0\",\"blockOwnerDeletion\":true,\"controller\":true,\"kind\":\"Instance\",\"name\":\"zigbee-abd677\",\"uid\":\"4d0bf3d6-a18c-4cdf-9dd7-27fa5cff22fe\"}]},\"spec\":{\"affinity\":{\"nodeAffinity\":{\"requiredDuringSchedulingIgnoredDuringExecution\":{\"nodeSelectorTerms\":[{\"matchFields\":[{\"key\":\"metadata.name\",\"operator\":\"In\",\"values\":[\"khadas\"]}]}]}}},\"containers\":[{\"env\":[{\"name\":\"TZ\",\"value\":\"Europe/Paris\"}],\"image\":\"koenkk/zigbee2mqtt:latest\",\"imagePullPolicy\":\"IfNotPresent\",\"name\":\"zigbee-broker\",\"ports\":[{\"containerPort\":8080,\"name\":\"http\",\"protocol\":\"TCP\"}],\"resources\":{\"limits\":{\"memory\":\"128Mi\"},\"requests\":{\"cpu\":\"100m\",\"memory\":\"128Mi\"}},\"securityContext\":{\"privileged\":true},\"volumeMounts\":[{\"mountPath\":\"/app/data\",\"name\":\"workdir\"}]}],\"initContainers\":[{\"command\":[\"cp -f /tmp/configuration.yaml /workdir/configuration.yaml\"],\"image\":\"tetragramato/kubectl-jq:0.1.0\",\"imagePullPolicy\":\"Always\",\"name\":\"zigbee-init\",\"resources\":{\"limits\":{\"{{PLACEHOLDER}}\":\"1\"},\"requests\":{\"{{PLACEHOLDER}}\":\"1\"}},\"volumeMounts\":[{\"mountPath\":\"/tmp/configuration.yaml\",\"name\":\"config\",\"subPath\":\"configuration.yaml\"},{\"mountPath\":\"/workdir\",\"name\":\"workdir\"}]}],\"volumes\":[{\"configMap\":{\"name\":\"zigbee2mqtt-config\"},\"name\":\"config\"},{\"emptyDir\":{},\"name\":\"workdir\"}]}}")] returned kube error: ErrorResponse { status: "Failure", message: "Pod \"zigbee-abd677-pod\" is invalid: [spec.initContainers[0].resources.limits[{{PLACEHOLDER}}]: Invalid value: \"{{PLACEHOLDER}}\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]'), spec.initContainers[0].resources.limits[{{PLACEHOLDER}}]: Invalid value: \"{{PLACEHOLDER}}\": must be a standard resource for containers, spec.initContainers[0].resources.requests[{{PLACEHOLDER}}]: Invalid value: \"{{PLACEHOLDER}}\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]'), spec.initContainers[0].resources.requests[{{PLACEHOLDER}}]: Invalid value: \"{{PLACEHOLDER}}\": must be a standard resource for containers]", reason: "Invalid", code: 422 }
thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: Pod "zigbee-abd677-pod" is invalid: [spec.initContainers[0].resources.limits[{{PLACEHOLDER}}]: Invalid value: "{{PLACEHOLDER}}": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]'), spec.initContainers[0].resources.limits[{{PLACEHOLDER}}]: Invalid value: "{{PLACEHOLDER}}": must be a standard resource for containers, spec.initContainers[0].resources.requests[{{PLACEHOLDER}}]: Invalid value: "{{PLACEHOLDER}}": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]'), spec.initContainers[0].resources.requests[{{PLACEHOLDER}}]: Invalid value: "{{PLACEHOLDER}}": must be a standard resource for containers]: Invalid', controller/src/util/instance_action.rs:83:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: JoinError::Panic(Id(21), ...)', controller/src/main.rs:48:64
Error: JoinError::Panic(Id(3), ...)

Additional context
Add any other context about the problem here.

@Tetragramato Tetragramato added the bug Something isn't working label Aug 9, 2023
@Tetragramato Tetragramato changed the title BrokerPodSpec and initContainers BrokerPodSpec and initContainers resources PLACEHOLDER Aug 9, 2023
@diconico07
Copy link
Contributor

I took a look on this, it seems we only go through the containers field to replace the {{PLACEHOLDER}}, I'll do a quick PR next week.

@diconico07 diconico07 self-assigned this Aug 18, 2023
@Tetragramato
Copy link
Author

Tetragramato commented Aug 18, 2023

Thanks, yeah i looked the code and saw tests only on containers list. Btw i didn't saw the code concerned by the env vars injection 🤔

Is it a hook ? (i don't know if you can use hook for initContainers 🤔 )

I'm curious to see where is it, if you can link the code concerned ?

Thanks in advance ;)

@jbpaux
Copy link
Contributor

jbpaux commented Aug 18, 2023

I think it's there:

akri/shared/src/k8s/pod.rs

Lines 229 to 252 in 5fbc942

for container in &mut pod_spec.containers {
if let Some(resources) = container.resources.as_ref() {
container.resources = Some(ResourceRequirements {
limits: {
match resources.limits.clone() {
Some(mut map) => {
insert_akri_resources(&mut map);
Some(map)
}
None => None,
}
},
requests: {
match resources.requests.clone() {
Some(mut map) => {
insert_akri_resources(&mut map);
Some(map)
}
None => None,
}
},
});
};
}

diconico07 added a commit to diconico07/akri that referenced this issue Aug 21, 2023
Broker pods can have init containers that need access to akri resources.

This commit adds the logic to replace the `{{PLACEHOLDER}}` for init
containers in the same way it is done for regular containers.

Fix project-akri#645

Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
diconico07 added a commit that referenced this issue Sep 4, 2023
* fix: Replace placeholder for initContainers in broker pods

Broker pods can have init containers that need access to akri resources.

This commit adds the logic to replace the `{{PLACEHOLDER}}` for init
containers in the same way it is done for regular containers.

Fix #645

Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>

* Update patch version

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

---------

Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants