[OCTRL-1091] Integrate k8s client into ECS core - #833
Conversation
29df8e4 to
d161264
Compare
knopers8
left a comment
There was a problem hiding this comment.
Thanks, i have comments mostly to naming, while the logic and approach seems OK for me.
task/manager.go becomes less clean due to placing kubernetes stuff there, but I wouldn't worry too much with creating a common abstraction for k8s + mesos, since this code is subject to further rewriting or starting it from scratch without mesos in the first place.
| m.roster.append(e.task) | ||
| } | ||
|
|
||
| envCRDName, err := m.createK8sEnvironmentCRD(ctx, envId, nodeToRefs) |
There was a problem hiding this comment.
I might be mistaken, but isn't it a custom object that we are creating, not Custom Resource Definition? My understanding is that a CRD defines what fields our custom objects should have at the new custom resource/endpoint.
| envCRDName, err := m.createK8sEnvironmentCRD(ctx, envId, nodeToRefs) | |
| envCRDName, err := m.createK8sEnvironmentObject(ctx, envId, nodeToRefs) |
So if I am right, any occurence of "CRD" should be replaced here with "Object" or removed.
There was a problem hiding this comment.
no this is actual CRD that we are creating and filling in
There was a problem hiding this comment.
Well, doesn't that happen during make install? Why do we have to do it at environment creation?
Control/control-operator/Makefile
Line 202 in 763366e
There was a problem hiding this comment.
Ah, I understand now. No I am not installing anything, cluster alreay needs to know about this CRD to create an instance of it (that is what install is for). But this code does the same as what you do if you use kubectl apply -f environment.yaml. It just describes the instance of environment by filling in the instance of Environment CRD with required tasks and other properties and applies it to the cluster.
There was a problem hiding this comment.
Exactly. So my point was that Environment CRD is what we create by kubectl apply during make install. As in the doc:
Defining a CRD object creates a new custom resource with a name and schema that you specify.
Then:
A resource is an endpoint in the Kubernetes API that stores a collection of API objects of a certain kind; for example, the built-in pods resource contains a collection of Pod objects.
So what we are creating here is an object (instance) of the Environment custom resource.
There was a problem hiding this comment.
yes, that is what I am saying and the reason for the name. If you use just "Object" it is misleading as in "what object" it is concretly defined as instance of CRD... I can rename it to createK8sEnvironmentCRDInstance, but I think that leaving CRD at all from the name is obscuring information from reader
There was a problem hiding this comment.
I changed naming to what we agreed in person
60eee7b to
1701f54
Compare
1701f54 to
9b1f04a
Compare
|
Sorry for the noisy force-pushing. I was resolving conflicts before we decided on naming. But the change of naming is in it's own commit, so it should be easy to review. |
This is actual implementation of the ECS Kubernetes bridge.
Before merging it needs the OCTRL-1090
The goal of this PR was to take existing code that deploys Mesos tasks and adjusts it for the kubernetes client usage, basically linking CRDs defined and managed by managers in control-operator to the existing ECS task infrastructure and classes. The code from managerk8s.go was inspired roughly accroding to this table (generated by llm, but I went through it):
managerk8s.gocreateK8sTaskEntries— InboundChannels loop, IPC endpoint,::globalaliasmakeTaskForMesosResources— same loop, IPC/TCP handling, global aliasscheduler.go:1346buildK8sNodeTaskRefs—O2_ROLE/O2_SYSTEMenv var injection loopmakeTaskForMesosResources—fillEnvDefaultloop for the same two varsscheduler.go:1458buildK8sNodeTaskRefs— ArgsCLI trimming/filteringmakeTaskForMesosResources—cmd.Argumentshandlingscheduler.go:1437newTaskForKubernetesnewTaskForMesosOffermanager.go:169deployKubernetesTasks— overall flowacquireTasks— Mesos deployment blockmanager.go:550configureK8sTasks—BuildPropertyMapper task, global bind mapconfigureTasks— bind map building +BuildPropertyMapsmanager.go:804transitionAndWaitK8sEnvState— send transition, block until state reachedtransitionTasks— send Mesos command, block on responsemanager.go:901killK8sTaskdoKillTaskmanager.go:1238killK8sEnvironmentreleaseTasksmanager.go:733process— route watch event to roster task (called fromwatchK8sTasks)updateTaskState/updateTaskStatusmanager.go:1071watchK8sTasks— retry loop on watcher failurescheduler.gonodeNameFromDescriptor— scan constraints formachine_idSatisfy(targetConstraints)scheduler.go:743k8sEnvRegistry,createK8sEnvironmentCRD,waitForK8sEnvState,cmdEnvToK8sEnvVarsIt contains multiple changes:
controlmodes to distinguish between Kubernetes and Mesos tasksreadout,stfbuilder-senderoutputandstfsender