Plumb rest config through from manager. #1334
Conversation
Plumb the config through alongside the client object (since they convey the same information really) all the way to pipe task. This way we use the same explicit config everywhere, rather than rely on config.GetConfig to guess the correct one. This is especially important when running the controller outside of a cluster.
porridge
requested review from
alenkacz,
gerred,
jbarrick-mesosphere,
kensipe,
nfnt and
zen-dog
as code owners
| @@ -15,6 +16,7 @@ import ( | |||
| // Context is a engine.task execution context containing k8s client, templates parameters etc. | |||
| type Context struct { | |||
| Client client.Client | |||
| Config *rest.Config | |||
There was a problem hiding this comment.
Should this be a rest.Config and not a pointer? I'm not too sure about the go semantics when to use a pointer and when not, but a pointer indicates that the var can be modified?
Not a blocker for me though.
There was a problem hiding this comment.
I don't think using a pointer or not has any impact on mutability.
rest.Config is a struct, so one uses a pointer when you don't want to make a copy.
This seems consistent with how I see this object type being used elsewhere.
There was a problem hiding this comment.
The struct member can always be modified here so this isn't a blocker. This is correct. For embedded structs, typically it's better to copy the ref rather than copy the entire struct (with exceptions). @ANeumann82 as a note for other code as well, interfaces are ALWAYS written as the interface value (not with a * for the pointer), but are ALWAYS a reference.
Plumb the config through alongside the client object (since they convey the same information really) all the way to pipe task. This way we use the same explicit config everywhere, rather than rely on config.GetConfig to guess the correct one. This is especially important when running the controller outside of a cluster. Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Plumb the config through alongside the client object (since they convey the same information really) all the way to pipe task. This way we use the same explicit config everywhere, rather than rely on config.GetConfig to guess the correct one. This is especially important when running the controller outside of a cluster. Signed-off-by: Thomas Runyon <runyontr@gmail.com>
What this PR does / why we need it:
Plumb the config through alongside the client object (since they convey
the same information really) all the way to pipe task.
This way we use the same explicit config everywhere, rather than rely on
config.GetConfig to guess the correct one. This is especially important
when running the controller outside of a cluster.
Fixes kudobuilder/operators#205