fix: update the target service deployment spec after the plugin execution #57
Conversation
| @@ -266,7 +265,7 @@ func applyPatch( | |||
| } | |||
|
|
|||
| // if the service is an external service of the target service, it was already handled above | |||
| if lo.Contains(externalServices, service) && !lo.Contains(alreadyHandledExternalServices, service.ServiceID) { | |||
There was a problem hiding this comment.
"alreadyHandledExternalServices" is never filled.
I think the service handled above doesn't fall under this condition because "lo.Contains" is comparing pointers and the service pointer has changed after that modification above
There was a problem hiding this comment.
is this still true after the new gateway support? let me check it
| @@ -206,7 +204,8 @@ func applyPatch( | |||
| } | |||
|
|
|||
| modifiedTargetService := DeepCopyService(targetService) | |||
| modifiedTargetService.DeploymentSpec = deploymentSpec | |||
| // TODO: find a better way to update deploymentSpec, this assumes there is only container in the pod | |||
| modifiedTargetService.DeploymentSpec.Template.Spec.Containers[0].Image = newImage | |||
There was a problem hiding this comment.
the target service deployment spec could have been updated when executing the external plugins before these lines, for instance, there could be some env vars overwriting that we need to keep in the deployment spec.
The new image is the only value we need to maintain from the flow_patch.
There was a problem hiding this comment.
maybe we should think on a MergeSpec(a Spec, b Spec) Spec function instead. Then we decide how to merge it inside the function (like a always take precedence)
| Service: devServiceName, | ||
| DeploymentSpec: deploymentSpec, | ||
| Service: devServiceName, | ||
| Image: item.Image, |
There was a problem hiding this comment.
Why use the image instead of the spec?
| @@ -266,7 +265,7 @@ func applyPatch( | |||
| } | |||
|
|
|||
| // if the service is an external service of the target service, it was already handled above | |||
| if lo.Contains(externalServices, service) && !lo.Contains(alreadyHandledExternalServices, service.ServiceID) { | |||
There was a problem hiding this comment.
is this still true after the new gateway support? let me check it
| @@ -206,7 +204,8 @@ func applyPatch( | |||
| } | |||
|
|
|||
| modifiedTargetService := DeepCopyService(targetService) | |||
| modifiedTargetService.DeploymentSpec = deploymentSpec | |||
| // TODO: find a better way to update deploymentSpec, this assumes there is only container in the pod | |||
| modifiedTargetService.DeploymentSpec.Template.Spec.Containers[0].Image = newImage | |||
There was a problem hiding this comment.
maybe we should think on a MergeSpec(a Spec, b Spec) Spec function instead. Then we decide how to merge it inside the function (like a always take precedence)
No description provided.