expectedServices function not copying desired spec

[bhiravabhatla]

opentelemetry-operator/pkg/collector/reconcile/service.go

Lines 182 to 199 in db40230

	updated := existing.DeepCopy()
	if updated.Annotations == nil {
	updated.Annotations = map[string]string{}
	}
	if updated.Labels == nil {
	updated.Labels = map[string]string{}
	}
	updated.ObjectMeta.OwnerReferences = desired.ObjectMeta.OwnerReferences
	for k, v := range desired.ObjectMeta.Annotations {
	updated.ObjectMeta.Annotations[k] = v
	}
	for k, v := range desired.ObjectMeta.Labels {
	updated.ObjectMeta.Labels[k] = v
	}
	patch := client.MergeFrom(&params.Instance)

I guess, we should be copying desired.Spec to updated.Spec if I am not wrong. -

  updated.Spec = desired.Spec

If we dont do that service spec wont be updated if some one adds a new receiver in config or explicit port in Spec.Ports of collector.

[jpkrohling]

From what I remember, we can't simply copy the Spec, as the clusterIP has to either be empty or None. The service controller will then change the value to a real IP if it's empty, and an update of this resource will fail if we specify the IP ourselves. So, I prefer to cherry-pick the properties that we want to override.

[bhiravabhatla]

Yep. I forgot about Cluster IP. we cant copy the whole spec. But I think we should copy spec.Ports atleast - or else we would miss out on any new ports added

[jpkrohling]

Could you clarify on "ports added"? Do you mean that users might add ports to a service?

[bhiravabhatla]

I meant some one can add a new receiver in opentelemetry collector CR config or a new port in CR spec.ports

[jpkrohling]

Right, the new desired ports should override the current list of ports.

[bhiravabhatla]

Yep :).

[thib92]

Should we only copy the ports? Otherwise, which fields of the Service spec would we wish to copy?

[jpkrohling]

Here are the reference docs for the Service object. I think copying the ports should be sufficient, but review it as well and see if you have another suggestion.

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#service-v1-core