Repository navigation
K8s core arguments - #865
Conversation
knopers8
left a comment
knopers8
left a comment
There was a problem hiding this comment.
In general, I feel that enabling the client by setting task namespace seems quite unobvious. I would propose to have a separate flag for enabling k8s support. Then, we can fail the core startup if the client is provided with wrong arguments and with the enable flag.
Also, I don't think we already need to enable kubernetes support by default, since we still have a production setup which does not work with kubernetes. I would propose to keep it disabled by default.
| if k8sClient, k8sErr := newK8sClientFromViper(); k8sErr != nil { | ||
| log.WithField("level", infologger.IL_Devel).WithError(k8sErr).Warn("K8s client init failed, K8s tasks disabled") | ||
| } else if k8sClient == nil { | ||
| log.Info("kubeNamespace not set, K8s tasks disabled") |
There was a problem hiding this comment.
I suspect there could be more than one reason for newK8sClientFromViper to return a nil client, if not currently, then at some point in the future. Then, the message might become misleading if we forget to update it.
This else-if branch could be removed if you agree to add an enable/disable flag for k8s support.
| If the Kubernetes client cannot be created (e.g. the kubeconfig file doesn't exist), core logs a warning `K8s client init failed, K8s tasks disabled` | ||
| and continues running with Mesos only. Deploying a task with a Kubernetes control mode then fails. |
There was a problem hiding this comment.
Do we really want to document the exact logs that are produced by the core? I am almost certain we will forget to update it when changed.
Anyway, one could remove this paragraph if enable flag for k8s is added.
|
|
||
| ### Core configuration | ||
|
|
||
| The Kubernetes client in ECS core is configured with the following parameters. Like any other core parameter they can be passed as CLI arguments |
There was a problem hiding this comment.
| The Kubernetes client in ECS core is configured with the following parameters. Like any other core parameter they can be passed as CLI arguments | |
| The Kubernetes client in ECS core is configured with the following parameters, passed as CLI arguments or in placed in `o2/components/aliecs/ANY/any/settings` consul file: |
proposal to mention both
| ### TaskTemplates | ||
|
|
||
| Since some tasks are deployed with known values ahead of time (image, env vars, ...), we added the `TaskTemplate` CR, which stores this data in the cluster so the user doesn't have to define everything every time. You can find all the `TaskTemplate`s in the `ecs-manifests/task-templates` folder. Currently we have templates for DPL, Readout, and StfBuilder/Sender. These templates must be applied to the cluster for the ECS bridge to work. | ||
|
|
||
| ### DPL and JIT | ||
|
|
||
| DPL tasks work similarly to those mentioned before: they use `kubernetes_fairmq` in their workflow YAML manifests. However, there is a problem: JIT tasks' manifests are created the moment they are run. So for now you need to first run the workflow to generate those tasks, then change the control mode after they've been generated. This will be changed in the future. | ||
|
|
||
| ### Worflow templates |
There was a problem hiding this comment.
TaskTemplates, DPL and JIT and (empty) Workflow templates do not seem to belong to this PR.
|
As you wish. I don't see a need for specific toggle to enable k8s given that they have totally different path parallel to mesos and you need to setup specific type in yaml templates. But if you feel that this is not enough and we require specific toggle to enable whole k8s, I will do so |
|
I am not fearing that the enabled K8s paths might break something in the mesos paths. As I mentioned, I mostly don't like the fact that K8s support is now enabled by having a value for an argument with a name which does not suggest enabling K8s ( |
|
sorry the comment should have been accompanied with the changes, but I messed up a bit by pushing the changes later |
2 participants
I addressed correctly pointed out inconsistencies with k8s core CLI arguments and added documentation