Skip to content

Commit

Permalink
Avoid assigning nil value to required field in podSpec: Containers (#317
Browse files Browse the repository at this point in the history
)

* Avoid assigning nill value to required field: Containers

* nit: refactor  comment
  • Loading branch information
SupriyaKasten authored and mergify[bot] committed Sep 24, 2019
1 parent eb7dcde commit 1c295f7
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion pkg/kube/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,19 @@ func WaitForPodCompletion(ctx context.Context, cli kubernetes.Interface, namespa

// PodSpecOverride override default pod Spec with the ones provided via specs
func PodSpecOverride(ctx context.Context, defaultSpecs, overrideSpecs v1.PodSpec) (v1.PodSpec, error) {
containers := defaultSpecs.Containers
// - Marshal override specs
// - Unmarshal override specs on default object so that it overrides only the fields that are present in override specs
override, err := json.Marshal(overrideSpecs)
if err != nil {
return v1.PodSpec{}, err
}
// - Unmarshal override specs on default object so that it overrides only the fields that are present in override specs

This comment has been minimized.

Copy link
@julio-lopez

julio-lopez Sep 24, 2019

Contributor

I am confused here, partly by this comment and also because I do not completely understand what the contract (or expected behavior) for this function is.
After the Unmarshal below, defaultSpecs will have the exact same contents of overrideSpecs, and then defaultSpecs.Containers is overwritten when override.Containers was nil. Is that the intended behavior?

@SupriyaKasten Could you provide some context?

err = json.Unmarshal(override, &defaultSpecs)
if err != nil {
return v1.PodSpec{}, err
}
if defaultSpecs.Containers == nil {
defaultSpecs.Containers = containers
}
return defaultSpecs, nil
}

0 comments on commit 1c295f7

Please sign in to comment.