Adding primaryContainerName implementation to podBuilder

[niliayu]

TL;DR

flyteorg/flyte#2703

Type

• Bug Fix
• Feature
• Plugin

Are all requirements met?

• Code completed
• Smoke tested
• Unit tests added
• Code documentation added
• Any pending items have an associated Issue

Complete description

Most of the detailed discussion leading up to this change is in this slack thread.

Tracking Issue

fixes flyteorg/flyte#2703

Follow-up issue

NA

TODO (all completed):

• Need to replace 'default' name with something real
• Need to refactor sidecarPodBuilder.updatePodMetadata to use the new naming
• Test everything

[welcome]

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

• Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
• Sign off your commits (Reference: DCO Guide).

[niliayu]

I've got this running with a pod-template, here's an example pod that was created with these changes:

Example pod (formatted terribly here, need to view raw or copy/paste somewhere that doesn't ignore indentation)

apiVersion: v1 kind: Pod metadata: name: a4lmk5ws6qkfbhx94dxc-n0-0 namespace: flytesnacks-development uid: 68e6dd69-4162-435f-ac1e-4c0da75aca07 resourceVersion: '187993' creationTimestamp: '2022-08-05T21:49:22Z' labels: domain: development execution-id: a4lmk5ws6qkfbhx94dxc interruptible: 'false' node-id: n0 project: flytesnacks shard-key: '1' task-name: flyte-project-example-task workflow-name: flyte-project-example-workflow annotations: cluster-autoscaler.kubernetes.io/safe-to-evict: 'false' primary_container_name: a4lmk5ws6qkfbhx94dxc-n0-0 ownerReferences: - apiVersion: flyte.lyft.com/v1alpha1 kind: flyteworkflow name: a4lmk5ws6qkfbhx94dxc uid: 9450a296-d243-4c78-a8f3-7ffc1e3e6b33 controller: true blockOwnerDeletion: true finalizers: - flyte/flytek8s selfLink: /api/v1/namespaces/flytesnacks-development/pods/a4lmk5ws6qkfbhx94dxc-n0-0 status: phase: Pending conditions: - type: PodScheduled status: 'False' lastProbeTime: null lastTransitionTime: '2022-08-05T21:49:22Z' reason: Unschedulable message: '0/1 nodes are available: 1 Insufficient cpu.' qosClass: Guaranteed spec: volumes: - name: apmsocketpath hostPath: path: /var/run/datadog/ type: '' - name: kube-api-access-r486f projected: sources: - serviceAccountToken: expirationSeconds: 3607 path: token - configMap: name: kube-root-ca.crt items: - key: ca.crt path: ca.crt - downwardAPI: items: - path: namespace fieldRef: apiVersion: v1 fieldPath: metadata.namespace defaultMode: 420 containers: - name: default image: docker.io/rwgrim/docker-noop args: - pyflyte-fast-execute - '--additional-distribution' - >- s3://my-s3-bucket/xs/flytesnacks/development/TDUWBF5VL7DUC4LC6UBRL5A4YE======/fast16282b6fa84bc0e633804f194037d1eb.tar.gz - '--dest-dir' - . - '--' - pyflyte-execute - '--inputs' - >- s3://my-s3-bucket/metadata/propeller/flytesnacks-development-a4lmk5ws6qkfbhx94dxc/n0/data/inputs.pb - '--output-prefix' - >- s3://my-s3-bucket/metadata/propeller/flytesnacks-development-a4lmk5ws6qkfbhx94dxc/n0/data/0 - '--raw-output-data-prefix' - s3://my-s3-bucket/test/5t/a4lmk5ws6qkfbhx94dxc-n0-0 - '--checkpoint-path' - s3://my-s3-bucket/test/5t/a4lmk5ws6qkfbhx94dxc-n0-0/_flytecheckpoints - '--prev-checkpoint' - '""' - '--resolver' - flytekit.core.python_auto_container.default_task_resolver - '--' - task-module - flyte.project.example - task-name - example_task env: - name: FLYTE_INTERNAL_EXECUTION_WORKFLOW value: >- flytesnacks:development:flyte.project.example.example_workflow - name: FLYTE_INTERNAL_EXECUTION_ID value: a4lmk5ws6qkfbhx94dxc - name: FLYTE_INTERNAL_EXECUTION_PROJECT value: flytesnacks - name: FLYTE_INTERNAL_EXECUTION_DOMAIN value: development - name: FLYTE_ATTEMPT_NUMBER value: '0' - name: FLYTE_INTERNAL_TASK_PROJECT value: flytesnacks - name: FLYTE_INTERNAL_TASK_DOMAIN value: development - name: FLYTE_INTERNAL_TASK_NAME value: flyte.project.example.example_task - name: FLYTE_INTERNAL_TASK_VERSION value: manual-register2 - name: FLYTE_INTERNAL_PROJECT value: flytesnacks - name: FLYTE_INTERNAL_DOMAIN value: development - name: FLYTE_INTERNAL_NAME value: flyte.project.example.example_task - name: FLYTE_INTERNAL_VERSION value: manual-register2 - name: FLYTE_AWS_ACCESS_KEY_ID value: minio - name: FLYTE_AWS_SECRET_ACCESS_KEY value: miniostorage - name: AWS_METADATA_SERVICE_TIMEOUT value: '5' - name: AWS_METADATA_SERVICE_NUM_ATTEMPTS value: '20' - name: FLYTE_AWS_ENDPOINT value: http://minio.flyte:9000 resources: limits: cpu: '2' memory: 200Mi requests: cpu: '2' memory: 200Mi volumeMounts: - name: apmsocketpath mountPath: /var/run/datadog - name: kube-api-access-r486f readOnly: true mountPath: /var/run/secrets/kubernetes.io/serviceaccount terminationMessagePath: /dev/termination-log terminationMessagePolicy: File imagePullPolicy: Always - name: a4lmk5ws6qkfbhx94dxc-n0-0 image: gcr.io/pachama-labs/project/project:0.68.0 args: - pyflyte-fast-execute - '--additional-distribution' - >- s3://my-s3-bucket/xs/flytesnacks/development/TDUWBF5VL7DUC4LC6UBRL5A4YE======/fast16282b6fa84bc0e633804f194037d1eb.tar.gz - '--dest-dir' - . - '--' - pyflyte-execute - '--inputs' - >- s3://my-s3-bucket/metadata/propeller/flytesnacks-development-a4lmk5ws6qkfbhx94dxc/n0/data/inputs.pb - '--output-prefix' - >- s3://my-s3-bucket/metadata/propeller/flytesnacks-development-a4lmk5ws6qkfbhx94dxc/n0/data/0 - '--raw-output-data-prefix' - s3://my-s3-bucket/test/5t/a4lmk5ws6qkfbhx94dxc-n0-0 - '--checkpoint-path' - s3://my-s3-bucket/test/5t/a4lmk5ws6qkfbhx94dxc-n0-0/_flytecheckpoints - '--prev-checkpoint' - '""' - '--resolver' - flytekit.core.python_auto_container.default_task_resolver - '--' - task-module - flyte.project.example - task-name - example_task - pyflyte-fast-execute - '--additional-distribution' - >- s3://my-s3-bucket/xs/flytesnacks/development/TDUWBF5VL7DUC4LC6UBRL5A4YE======/fast16282b6fa84bc0e633804f194037d1eb.tar.gz - '--dest-dir' - . - '--' - pyflyte-execute - '--inputs' - >- s3://my-s3-bucket/metadata/propeller/flytesnacks-development-a4lmk5ws6qkfbhx94dxc/n0/data/inputs.pb - '--output-prefix' - >- s3://my-s3-bucket/metadata/propeller/flytesnacks-development-a4lmk5ws6qkfbhx94dxc/n0/data/0 - '--raw-output-data-prefix' - s3://my-s3-bucket/test/5t/a4lmk5ws6qkfbhx94dxc-n0-0 - '--checkpoint-path' - s3://my-s3-bucket/test/5t/a4lmk5ws6qkfbhx94dxc-n0-0/_flytecheckpoints - '--prev-checkpoint' - '""' - '--resolver' - flytekit.core.python_auto_container.default_task_resolver - '--' - task-module - flyte.project.example - task-name - example_task env: - name: FLYTE_INTERNAL_EXECUTION_WORKFLOW value: >- flytesnacks:development:flyte.project.example.example_workflow - name: FLYTE_INTERNAL_EXECUTION_ID value: a4lmk5ws6qkfbhx94dxc - name: FLYTE_INTERNAL_EXECUTION_PROJECT value: flytesnacks - name: FLYTE_INTERNAL_EXECUTION_DOMAIN value: development - name: FLYTE_ATTEMPT_NUMBER value: '0' - name: FLYTE_INTERNAL_TASK_PROJECT value: flytesnacks - name: FLYTE_INTERNAL_TASK_DOMAIN value: development - name: FLYTE_INTERNAL_TASK_NAME value: flyte.project.example.example_task - name: FLYTE_INTERNAL_TASK_VERSION value: manual-register2 - name: FLYTE_INTERNAL_PROJECT value: flytesnacks - name: FLYTE_INTERNAL_DOMAIN value: development - name: FLYTE_INTERNAL_NAME value: flyte.project.example.example_task - name: FLYTE_INTERNAL_VERSION value: manual-register2 - name: FLYTE_AWS_ACCESS_KEY_ID value: minio - name: FLYTE_AWS_SECRET_ACCESS_KEY value: miniostorage - name: AWS_METADATA_SERVICE_TIMEOUT value: '5' - name: AWS_METADATA_SERVICE_NUM_ATTEMPTS value: '20' - name: FLYTE_AWS_ENDPOINT value: http://minio.flyte:9000 - name: FLYTE_INTERNAL_EXECUTION_WORKFLOW value: >- flytesnacks:development:flyte.project.example.example_workflow - name: FLYTE_INTERNAL_EXECUTION_ID value: a4lmk5ws6qkfbhx94dxc - name: FLYTE_INTERNAL_EXECUTION_PROJECT value: flytesnacks - name: FLYTE_INTERNAL_EXECUTION_DOMAIN value: development - name: FLYTE_ATTEMPT_NUMBER value: '0' - name: FLYTE_INTERNAL_TASK_PROJECT value: flytesnacks - name: FLYTE_INTERNAL_TASK_DOMAIN value: development - name: FLYTE_INTERNAL_TASK_NAME value: flyte.project.example.example_task - name: FLYTE_INTERNAL_TASK_VERSION value: manual-register2 - name: FLYTE_INTERNAL_PROJECT value: flytesnacks - name: FLYTE_INTERNAL_DOMAIN value: development - name: FLYTE_INTERNAL_NAME value: flyte.project.example.example_task - name: FLYTE_INTERNAL_VERSION value: manual-register2 - name: FLYTE_AWS_ACCESS_KEY_ID value: minio - name: FLYTE_AWS_SECRET_ACCESS_KEY value: miniostorage - name: AWS_METADATA_SERVICE_TIMEOUT value: '5' - name: AWS_METADATA_SERVICE_NUM_ATTEMPTS value: '20' - name: FLYTE_AWS_ENDPOINT value: http://minio.flyte:9000 resources: limits: cpu: '2' memory: 200Mi requests: cpu: '2' memory: 200Mi volumeMounts: - name: kube-api-access-r486f readOnly: true mountPath: /var/run/secrets/kubernetes.io/serviceaccount terminationMessagePath: /dev/termination-log terminationMessagePolicy: FallbackToLogsOnError imagePullPolicy: IfNotPresent restartPolicy: Never terminationGracePeriodSeconds: 30 dnsPolicy: ClusterFirst serviceAccountName: default serviceAccount: default securityContext: {} schedulerName: default-scheduler tolerations: - key: node.kubernetes.io/not-ready operator: Exists effect: NoExecute tolerationSeconds: 300 - key: node.kubernetes.io/unreachable operator: Exists effect: NoExecute tolerationSeconds: 300 priority: 0 enableServiceLinks: true preemptionPolicy: PreemptLowerPriority ```

Next up I need to figure out a test workflow for sidecars, since my understanding of how those should look is still a bit high level, and I'm not very sure I've changed the sidecar code the way we want or not. Once I get that sorted I'll have a look at unit tests I should add for this.

[hamersaw]

Looks great so far! Left a few comments, I think you'll have to fix a few things in the sidecar plugin once you get to it. Here is documentation on defining a task to use the sidecar plugin.

[codecov]

Codecov Report

Merging #280 (3df404e) into master (e863c76) will increase coverage by 0.03%.
The diff coverage is 79.22%.

❗ Current head 3df404e differs from pull request most recent head 1468319. Consider uploading reports for the commit 1468319 to get more accurate results

@@            Coverage Diff             @@
##           master     #280      +/-   ##
==========================================
+ Coverage   63.28%   63.32%   +0.03%     
==========================================
  Files         145      145              
  Lines        9264     9311      +47     
==========================================
+ Hits         5863     5896      +33     
- Misses       2866     2872       +6     
- Partials      535      543       +8     

Flag	Coverage Δ
unittests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
go/tasks/plugins/k8s/pod/container.go	66.66% <60.00%> (-4.77%)	⬇️
go/tasks/pluginmachinery/flytek8s/pod_helper.go	78.94% <62.96%> (-2.34%)	⬇️
go/tasks/plugins/k8s/pod/sidecar.go	78.02% <81.81%> (-2.20%)	⬇️
go/tasks/pluginmachinery/encoding/encoder.go	100.00% <100.00%> (ø)
go/tasks/plugins/array/k8s/management.go	57.21% <100.00%> (ø)
go/tasks/plugins/k8s/pod/plugin.go	84.88% <100.00%> (+0.54%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[welcome]

Congrats on merging your first pull request! 🎉