feat: Introduce new KubeTaskParallel function to run pods with two containers #3095

hairyhum · 2024-09-06T21:18:19Z

Change Overview

In order to separate data dump generation by database tools and export by kando, a pod performing database dump can have two separate containers and set up the pipe over file instead of anonymous pipe.

The goal of this function is to enable database-specific blueprints to use official database images separately from kanister-tools image instead of building a custom image for each databse blueprint.

Pull request type

Please check the type of change your PR introduces:

Issues

fixes #issue-number

Test Plan

💪 Manual
⚡ Unit test
💚 E2E

hairyhum · 2024-09-06T21:19:21Z

There is an example in tests and docs.
Another example of blueprint:

apiVersion: cr.kanister.io/v1alpha1
kind: Blueprint
metadata:
  name: kube-task-parallel-bp
  namespace: kanister
actions:
  dump:
    phases:
    - func: KubeTaskParallel
      name: dump
      args:
        namespace: kanister
        sharedVolumeMedium: Memory
        sharedVolumeSizeLimit: 1Gi
        sharedVolumeDir: "/tmp/"
        backgroundImage: ubuntu:latest
        backgroundCommand:
        - bash
        - -o
        - errexit
        - -o
        - pipefail
        - -c
        - |
          mkfifo /tmp/pipe-file
          for i in {1..10}
          do
            echo $i
            sleep 1
          done > /tmp/pipe-file
        outputImage: ghcr.io/kanisterio/kanister-tools:v9.99.9-dev
        outputCommand:
        - bash
        - -o
        - errexit
        - -o
        - pipefail
        - -c
        - |
          while [ ! -e /tmp/pipe-file  ]; do sleep 1; done
          kando output numbers "$(cat /tmp/pipe-file)"

docs/functions.rst

viveksinghggits · 2024-09-09T16:33:38Z

Hi @hairyhum ,
Are we doing this to solve any performance issue that were reported? Can you please share some background just for my understanding.

hairyhum · 2024-09-09T17:13:53Z

Hi @hairyhum , Are we doing this to solve any performance issue that were reported? Can you please share some background just for my understanding.

The main goal of this change is to separate image management for databse dump blueprints. Not the performance.

hairyhum · 2024-09-11T16:10:17Z

Renamed function into KubeTaskParallel and the arguments for containers to background and output containers.

e-sumin

Overall LGTM, but for me function name looks a bit misleading.
But, frankly, I can't propose better name. Maybe other reviewers will have an ideas ?

pkg/function/kube_task_parallel.go

e-sumin

LGTM

viveksinghggits

looks good to me overall. just left some nit comments/formatting suggestion.

viveksinghggits · 2024-09-25T10:08:42Z

docs/functions.rst

@@ -150,6 +150,82 @@ Example:
        - |
          echo "Example"

+KubeTaskParallel


I think maybe we should the name of the function in next community call? It's good right now, but by reading the name it looks like in this function we are doing things parallelly that kubetask does sequentially, which doesn't seem to be the case right?
I think it doesn't necessarily have to have KubeTask in its name right? Maybe MultiContainerUpload or something in that line?

I was trying to avoid the "purpose" style names like "dump", "upload" or "export". While this function is used for that purpose it's deliberately made generic.

ok, let's take opinions from other folks as well. @pavannd1 @PrasadG193 @mlavi .

Somehow KubeTaskParallel doesn't sound good to me. I agree that it would be better if we avoid KubeTask* prefix for the new function.

We can use MultiContainerPod or MultiContainerRun, but we already use KubeTaslk instead of Pod in the other function.

viveksinghggits · 2024-09-25T10:10:20Z

docs/functions.rst

+It's similar to KubeTask, but allows using multiple images to move backup data.
+"background" container is one responsible for generating data, while "output" container
+should export it to destination.
+The main difference between them is that phase outputs can only generated from the


Suggested change

The main difference between them is that phase outputs can only generated from the

The main difference between these containers is that phase outputs can only be generated from the

nit

viveksinghggits · 2024-09-25T10:10:58Z

docs/functions.rst

+"background" container is one responsible for generating data, while "output" container
+should export it to destination.
+The main difference between them is that phase outputs can only generated from the
+"output" container outputs.


Suggested change

"output" container outputs.

"output" container.

?

viveksinghggits · 2024-09-25T10:17:43Z

docs/functions.rst

+   `backgroundImage`, Yes, `string`, image to be used in "background" container
+   `backgroundCommand`, Yes, `[]string`,  command list to execute in "background" container


should we call this container snapshot or backup container? or maybe data container? just a suggestion, if you think background is ok, I am good.

viveksinghggits · 2024-09-25T10:18:00Z

docs/functions.rst

+   `outputImage`, Yes, `string`, image to be used in "output" container
+   `outputCommand`, Yes, `[]string`,  command list to execute in "output" container


should this be called upload container?

Same note as a note before, trying to name things for what they do rather than what they can be used for.

viveksinghggits · 2024-09-25T10:26:01Z

releasenotes/notes/kube-task-parallel-function-d488516c0f3b22c6.yaml

@@ -0,0 +1,2 @@
+---
+features: Introduce new KubeTaskParallel function to run pods with two containers connected by shared volume


nit:

Suggested change

features: Introduce new KubeTaskParallel function to run pods with two containers connected by shared volume

features: Introduced new Kanister function ``KubeTaskParallel`` to run pods with two containers connected by shared volume

?

…ntainers In order to separate data dump generation by database tools and export by `kando`, a pod performing database dump can have two separate containers and set up the pipe over file instead of anonymous pipe

Init container can be used to set up data in the shared volume.

hairyhum force-pushed the dump-function branch from 31be024 to 32d48b2 Compare September 6, 2024 21:33

hairyhum requested review from mlavi, pavannd1 and viveksinghggits September 6, 2024 21:34

hairyhum force-pushed the dump-function branch 2 times, most recently from 7067c1e to ed6143f Compare September 9, 2024 15:56

viveksinghggits reviewed Sep 9, 2024

View reviewed changes

docs/functions.rst Outdated Show resolved Hide resolved

hairyhum force-pushed the dump-function branch from ed6143f to 8314e4a Compare September 9, 2024 17:11

hairyhum changed the title ~~feat: Introduce new Dump function to run pods with two containers~~ feat: Introduce new KubeTaskParallel function to run pods with two containers Sep 11, 2024

hairyhum force-pushed the dump-function branch from 0a792a0 to 8dbc0e2 Compare September 11, 2024 16:09

hairyhum force-pushed the dump-function branch 2 times, most recently from 514f8b2 to fdc0a1b Compare September 11, 2024 16:11

e-sumin reviewed Sep 13, 2024

View reviewed changes

pkg/function/kube_task_parallel.go Outdated Show resolved Hide resolved

hairyhum force-pushed the dump-function branch from dda5d4d to f3b2e13 Compare September 17, 2024 21:18

hairyhum requested review from e-sumin and viveksinghggits September 18, 2024 19:58

e-sumin approved these changes Sep 25, 2024

View reviewed changes

viveksinghggits reviewed Sep 25, 2024

View reviewed changes

hairyhum added 5 commits September 25, 2024 15:36

Code cleanup

7e6742f

Added init container support.

7952351

Init container can be used to set up data in the shared volume.

Fix docs warnings

719832b

Wording

19c31e8

hairyhum force-pushed the dump-function branch from 6bcc499 to 19c31e8 Compare September 25, 2024 19:37

Linter fixes

5a43532

hairyhum requested a review from viveksinghggits September 26, 2024 19:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Introduce new KubeTaskParallel function to run pods with two containers #3095

feat: Introduce new KubeTaskParallel function to run pods with two containers #3095

hairyhum commented Sep 6, 2024 •

edited

Loading

hairyhum commented Sep 6, 2024 •

edited

Loading

viveksinghggits commented Sep 9, 2024

hairyhum commented Sep 9, 2024

hairyhum commented Sep 11, 2024

e-sumin left a comment

e-sumin left a comment

viveksinghggits left a comment

viveksinghggits Sep 25, 2024 •

edited

Loading

hairyhum Sep 25, 2024

viveksinghggits Sep 26, 2024 •

edited

Loading

PrasadG193 Sep 27, 2024

hairyhum Sep 27, 2024

viveksinghggits Sep 25, 2024

viveksinghggits Sep 25, 2024

viveksinghggits Sep 25, 2024

viveksinghggits Sep 25, 2024

hairyhum Sep 25, 2024

viveksinghggits Sep 25, 2024

	The main difference between them is that phase outputs can only generated from the
	The main difference between these containers is that phase outputs can only be generated from the

		`backgroundImage`, Yes, `string`, image to be used in "background" container
		`backgroundCommand`, Yes, `[]string`, command list to execute in "background" container

		`outputImage`, Yes, `string`, image to be used in "output" container
		`outputCommand`, Yes, `[]string`, command list to execute in "output" container

		@@ -0,0 +1,2 @@
		---
		features: Introduce new KubeTaskParallel function to run pods with two containers connected by shared volume

	features: Introduce new KubeTaskParallel function to run pods with two containers connected by shared volume
	features: Introduced new Kanister function ``KubeTaskParallel`` to run pods with two containers connected by shared volume

feat: Introduce new KubeTaskParallel function to run pods with two containers #3095

Are you sure you want to change the base?

feat: Introduce new KubeTaskParallel function to run pods with two containers #3095

Conversation

hairyhum commented Sep 6, 2024 • edited Loading

Change Overview

Pull request type

Issues

Test Plan

hairyhum commented Sep 6, 2024 • edited Loading

viveksinghggits commented Sep 9, 2024

hairyhum commented Sep 9, 2024

hairyhum commented Sep 11, 2024

e-sumin left a comment

Choose a reason for hiding this comment

e-sumin left a comment

Choose a reason for hiding this comment

viveksinghggits left a comment

Choose a reason for hiding this comment

viveksinghggits Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viveksinghggits Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hairyhum commented Sep 6, 2024 •

edited

Loading

hairyhum commented Sep 6, 2024 •

edited

Loading

viveksinghggits Sep 25, 2024 •

edited

Loading

viveksinghggits Sep 26, 2024 •

edited

Loading