[OCTRL-1088] Environment Controller#816
Conversation
53d5a96 to
913d6af
Compare
knopers8
left a comment
There was a problem hiding this comment.
Thanks, I took some time to get acquainted with the news and finally tried running it today - it works!
I have some minor questions, but it can be merged already. Please put a human-readable title to the PR though.
| "strings" | ||
|
|
||
| pb "github.com/AliceO2Group/ControlOperator/internal/controller/protos/generated" | ||
| pb "github.com/AliceO2Group/Control/operator/internal/controller/protos/generated" |
There was a problem hiding this comment.
hmm, isn't it control-operator?
| pb "github.com/AliceO2Group/Control/operator/internal/controller/protos/generated" | |
| pb "github.com/AliceO2Group/Control/control-operator/internal/controller/protos/generated" |
There was a problem hiding this comment.
you are correct, I will address it in separate PR as proper renaming would touch multiple files and I can do at least once something properly in it's own PR/commit :D
| task.Spec.Control = *template.Spec.Control.DeepCopy() | ||
|
|
||
| if foundIdx := slices.IndexFunc(taskReference.Env, func(envVar v1.EnvVar) bool { return envVar.Name == "OCC_CONTROL_PORT" }); foundIdx == -1 { | ||
| log.Error(fmt.Errorf("didn't find OCC_CONTROL_PORT in env"), "failed to fill in env vars from template") |
There was a problem hiding this comment.
Shouldn't we return if there is error?
There was a problem hiding this comment.
yes, to have symetrical behavior with the other error handling in the controller... However I am not sure that it is the correct behavior here, as it drops the deployment, which might not be desired outcome. I added TODO into the code to address this potentialy
|
You mentioned in person that environment controller fails to relay transition arguments on your setup. When I tried on your minikube setup it indeed failed to configure tasks. However the same problem appeared when I run tasks individually without environment (using eg. |
Apart from environment controller I also created separate binaries for environment manager and task manager, which is the cause of most of the yaml piping.