kubectl replace doesn't work with immutable fields

[pbecotte]

kubectl replace doesn't work on service objects unless ClusterIP is specified. Not sure what the correct behavior should be? Came across this looking at issues with helm update --force not working if there is a service in the app.
https://github.com/helm/helm/issues/7350

#svc.yaml
kind: Service
apiVersion: v1
metadata:
  name: svc
spec:
  type: ClusterIP
  selector:
    app: myapp
  ports:
    - name: blah
      port: 8080

$ kubectl apply -f svc.yaml
service/svc configured
$ kubectl replace -f svc.yaml 
The Service "svc" is invalid: spec.clusterIP: Invalid value: "": field is immutable

[brianpursley]

I did a little research into this and have come to the conclusion this is likely a problem in the API.

Steps to reproduce using kubectl:

kubectl run nginx --image=nginx
kubectl expose deploy nginx --port=80 -o yaml --dry-run > svc.yaml
kubectl apply -f svc.yaml
kubectl replace -f svc.yaml

Error message (same as @pbecotte got)

The Service "nginx" is invalid: spec.clusterIP: Invalid value: "": field is immutable

Further investigation shows if I try to use the API directly to replace the service, I get the same error coming back from the API. For example:

kubectl proxy --port=8080 &
curl -X PUT http://localhost:8080/api/v1/namespaces/default/services/nginx  -H "Content-Type:application/json" -d '{"kind": "Service","apiVersion": "v1","metadata": {"name": "nginx","resourceVersion":"12","creationTimestamp": null,"labels": {"run": "nginx"}},"spec": {"ports": [{"protocol": "TCP","port": 80,"targetPort": 80}],"selector": {"run": "nginx"}},"status": {"loadBalancer": {}}}'

Response:

{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {

  },
  "status": "Failure",
  "message": "Service \"nginx\" is invalid: spec.clusterIP: Invalid value: \"\": field is immutable",
  "reason": "Invalid",
  "details": {
    "name": "nginx",
    "kind": "Service",
    "causes": [
      {
        "reason": "FieldValueInvalid",
        "message": "Invalid value: \"\": field is immutable",
        "field": "spec.clusterIP"
      }
    ]
  },
  "code": 422
}

So I'm guessing the services PUT endpoint needs to handle replacement of an existing service. It does not appear that replacing a service will work at all without the --force option, which deletes the service before making the PUT request.

I'll check the API issues and see if there is something there about this already and if not add an issue.

[soltysh]

This looks like it's not a problem with kubectl, you're getting a clear information back from the server that you can't modify immutable field.
/close

[k8s-ci-robot]

@soltysh: Closing this issue.

In response to this:

This looks like it's not a problem with kubectl, you're getting a clear information back from the server that you can't modify immutable field.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[pbecotte]

I don't understand...the issue report is very clearly saying that we aren't trying to modify the field. That's why I opened a bug report- very confusing to get that error message when you aren't trying to modify anything, and the "replace" command simply doesn't work on (at least) service objects.

[brianpursley]

I didn't mean to imply that the issue should definitely be closed, just that I was researching it a little more to find out if the problem is in the API. Sorry if I caused that confusion.

However, I checked the API code and they have a unit test explicitly checking that ClusterIP is not removed in an update call in pkg/apis/core/validation/validation_test.go:

{
    name: "remove cluster IP",
    tweakSvc: func(oldSvc, newSvc *core.Service) {
        oldSvc.Spec.ClusterIP = "1.2.3.4"
        newSvc.Spec.ClusterIP = ""
    },
    numErrs: 1,
}

So I see a few possible choices:

1. Do nothing. Make users of kubectl perform their own lookup of the ClusterIP from the service they want to update and update their yaml accordingly. This doesn't seem like an ideal user experience because that IP was assigned by Kubernetes, not the user and like @pbecotte said is confusing when you are not trying to explicitly change the ClusterIP.

2. Request a change to Kubernetes API to relax their ClusterIP immutability validation so that it will not fail if you have a missing ClusterIP. The assumption would be that the caller wants to use the same ClusterIP that is already assigned. Since the API project explicitly has a unit test checking against this, it makes me wonder if there is a reason it checks for that aside from kubectl's use case.

3. Change kubectl replace so that if you don't specify a ClusterIP, to populate it automatically using the service's current ClusterIP before making the PUT request to the API so that it won't reject the request. This seems like it could be the most straightforward to me.

@soltysh What are your thoughts. Would you consider re-opening to pursue option 3? Are there other options besides those I listed?

[thockin]

1. Do nothing. Make users of kubectl perform their own lookup of the ClusterIP from the service they want to update and update their yaml accordingly.

Think of this as read-modify-write - it's not an uncommon pattern.

1. Request a change to Kubernetes API to relax their ClusterIP immutability validation so that it will not fail if you have a missing ClusterIP. The assumption would be that the caller wants to use the same ClusterIP that is already assigned.

This is not impossible, but it turns PUT into PATCH. We have distinct PATCH operations for a reason, and that is almost certainly what you want in this caser (kubectl apply should "just work"). Is there a reason why you can't use PATCH or apply?

1. Change kubectl replace so that if you don't specify a ClusterIP, to populate it automatically using the service's current ClusterIP before making the PUT request

We don't really want to hand-roll client side changes for this. Then we'd need the same thing in client-go and everywhere else.

[thockin]

/assign

[pbecotte]

All three of those options seem like workarounds, with semantic problems as you mention. But they miss the point... PUT shouldn't depend on the status of what's currently in the slot. I would have expected PUT to work by removing the existing thing and then sticking a new one in it's place. Thats the whole point of using an option like "replace" instead of "apply". Are any other fields preserved from the new object if they aren't specified in the new?

[thockin]

We opted (rightly or wrongly) to NOT do an IP reallocation when someone
writes "" via PUT. Everyone agreed that was almost certainly not what a
user intended.

The bug here, if there is one, is that "" was accepted at all.

On Mon, Feb 8, 2021, 7:20 PM Paul Becotte [e-mail] wrote:

All three of those options seem like workarounds, with semantic problems
as you mention. But they miss the point... PUT shouldn't depend on the
status of what's currently in the slot. I would have expected PUT to work
by removing the existing thing and then sticking a new one in it's place.
Thats the whole point of using an option like "replace" instead of "apply".
Are any other fields preserved from the new object if they aren't specified
in the new?

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubectl/issues/798#issuecomment-775630323,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABKWAVCT6TUCSEKP7Q3QE6TS6CSZPANCNFSM4KE44CEA
.