Add Kubernetes native launcher#30
Conversation
…core-dev into yasirmaqsood-kubernetes
| { | ||
| // --------------------------------------------------------------- | ||
| // Instance properties | ||
| // --------------------------------------------------------------- |
There was a problem hiding this comment.
We try to avoid having more comments than needed, so remove this "instance properties" lines because it's evident what they are
| // Instance properties | ||
| // --------------------------------------------------------------- | ||
|
|
||
| /** @var string Kubernetes Job name, also used as the OpenVRE "pid" */ |
There was a problem hiding this comment.
Following the same principle, we might simplify these comments at some point but let's keep them because they are usefull right now
| // --------------------------------------------------------------- | ||
|
|
||
| /** @var string Kubernetes Job name, also used as the OpenVRE "pid" */ | ||
| private $pid = ""; |
There was a problem hiding this comment.
Change the class properties to have the type as part oh the property, not as part of the comment. E.g.
/** Kubernetes Job name, also used as the OpenVRE "pid" */
private string $pid = "";
This way the type is enforced automatically
| * @param array $jobOptions Options from Tooljob: "image" overrides container image, | ||
| * "env" provides extra environment variables for the pod | ||
| */ | ||
| public function __construct($cl = false, $workDir = "", $queue = "", $jobname = "", $cpu = 1, $mem = 0, $logFile = "job_output.log", $errFile = "job_error.log", $jobOptions = array()) |
There was a problem hiding this comment.
Remove the parameters that are not used: queue, logFIle and errFile
| * @param array $jobOptions Options from Tooljob: "image" overrides container image, | ||
| * "env" provides extra environment variables for the pod | ||
| */ | ||
| public function __construct($cl = false, $workDir = "", $queue = "", $jobname = "", $cpu = 1, $mem = 0, $logFile = "job_output.log", $errFile = "job_error.log", $jobOptions = array()) |
There was a problem hiding this comment.
Same as beofre for the types, add them to the contruct function and remove from the comments
There was a problem hiding this comment.
In all the functions with parameters: add the types to the contruct function and remove from the comments
| " --out_metadata " . $this->stageout_file_virtual . | ||
| " --log_file " . $this->log_file_virtual; | ||
|
|
||
| if (isset($this->launcher) && $this->launcher === "kubernetes_native") { |
There was a problem hiding this comment.
The launcher kubernetes_native never does not call this function setBashCommandDockerSge so this condition cannot happen
| RUN curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer | ||
| RUN composer self-update | ||
| RUN composer update --ignore-platform-req=ext-mongodb --ignore-platform-req=ext-mongodb | ||
| # Dependencies are pre-seeded in ./openVRE/vendor for offline/reproducible builds. |
There was a problem hiding this comment.
Why removing this? The dependencies are defined in composer.json and are versioned, so the builds should be reproducible
| #RUN groupmod -g $DOCKER_GROUP docker | ||
| #RUN usermod -aG docker application | ||
|
|
||
| ARG DOCKER_GROUP=1002 |
There was a problem hiding this comment.
This is already in the .env file
| else \ | ||
| groupadd -g "${DOCKER_GROUP}" docker; \ | ||
| fi; \ | ||
| usermod -aG docker application |
There was a problem hiding this comment.
The docker group already exists at this point so I would keep the old code
2 participants
Solved conflicts from #29