Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

creatOrReplace() should ignore if identical to last submitted: #2060

Closed
chirlo opened this issue Mar 12, 2020 · 19 comments
Closed

creatOrReplace() should ignore if identical to last submitted: #2060

chirlo opened this issue Mar 12, 2020 · 19 comments
Labels

Comments

@chirlo
Copy link

chirlo commented Mar 12, 2020

Server Version: version.Info{Major:"1", Minor:"13", GitVersion:"v1.13.3", GitCommit:"721bfa751924da8d1680787490c54b9179b1fed0", GitTreeState:"clean", BuildDate:"2019-02-01T20:00:57Z", GoVersion:"go1.11.5", Compiler:"gc", Platform:"linux/amd64"}

I've had this problem with a PersistentVolumeClaim but it might apply to others. I initally createOrReplace the following resource from a yaml file:

---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: my-pvc
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 20Gi

which succeeds and everything runs fine. Now If i try it again I right in the following error:

  io.fabric8.kubernetes.client.KubernetesClientException: ...  Received status: Status(apiVersion=v1, code=422, details=StatusDetails(causes=[StatusCause(field=spec, message=Forbidden: is immutable after creation except resources.requests for bound claims, reason=FieldValueForbidden, additionalProperties={})], group=null, kind=PersistentVolumeClaim, name=postgres, retryAfterSeconds=null, uid=null, additionalProperties={}), kind=Status, message=PersistentVolumeClaim "postgres" is invalid: spec: Forbidden: is immutable after creation except resources.requests for bound claims, metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=Invalid, status=Failure, additionalProperties={}).

This I understand is normal following a PUT, the issue is that it should haven't been PUT as it is the same resource that already exists.
I tracked the problem down to

The ResourceCompare.equals compares the item to be created with the reloaded item, which in this case has been added some spec entries by the K8s cluster:

volumeMode=Filesystem
volumeName=my-pv

The equals then returns false because of this added entries, and the item is then pushed although it should have been ignored.

I've also observed the reloaded item includes a metadata.annotations with the last applied configuration in JSON form:

"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"v1\",\"kind\":\"PersistentVolumeClaim\",\"metadata\":{\"annotations\":{},\"name\":\"postgres\",\"namespace\":\"default\"},\"spec\":{\"accessModes\":[\"ReadWriteOnce\"],\"resources\":{\"requests\":{\"storage\":\"20Gi\"}}}}\n"

Without any knowledge of Kubernetes internals, I'd guess that that JSON string is what needs to be compared in the 'equals' method against the item to be deployed.
This seem to be confirmed here: How apply calculates differences and merges changes

I can try to go for a PR if you think this is the way to go

@manusa manusa added the bug label Apr 30, 2020
@nbn
Copy link

nbn commented May 5, 2020

Hi, I may be facing the same issues, having a service with a spec.type=ClusterIP. I can create it, but subsequently it fails with immutable field spec.clusterIP, although the spec is unaltered and does not specify the clusterIP.

at io.fabric8.kubernetes.client.handlers.ServiceHandler.replace(ServiceHandler.java:39) at io.fabric8.kubernetes.client.dsl.internal.NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.createOrReplace(NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java:271)

Not must else information to provide, other than I use the 4.10.0 version of the kubernetes-client and openshift-client jar.

@rohanKanojia
Copy link
Member

@nbn : spec.clusterIP gets populated on the first time you create it by kubernetes apiserver. Next createOrReplace() call goes as a PUT which gets rejected. Am I missing something here?

@nbn
Copy link

nbn commented May 5, 2020

@rohanKanojia I just ran a debug session. My input does not have a spec.clusterIP set explicitly, so yes it gets created the first time. Second time, it Use ResourceCompare.equals(r,meta) and I can see r having a spec.IP, while meta's is ´null´. Then it calls replace, which funny enough only seems to be updating the ´spec.clusterIP` and then it fails.

@rohanKanojia
Copy link
Member

I remember I fixed an issue similar to this in #1931 . Are you saying client patching spec.clusterIP is causing problems?

@nbn
Copy link

nbn commented May 5, 2020

I can only tell you what I observe. I try to roll the same spec through twice, and the second time it fails. I read a yaml list (Having it as a list) and then simply invoke it with:

client.resourceList(itemsList).inNamespace(aNamespace).createOrReplace()

The service is simple enough, though

apiVersion: v1
kind: Service
metadata:
  labels:
  name: my-service
spec:
  ports:
  - name: http
    port: 80
    protocol: TCP
    targetPort: http
  selector:
    artifact: mypod
  sessionAffinity: None
  type: ClusterIP

And it is an openshift v3.11.170

@rohanKanojia
Copy link
Member

@nbn : Will try to reproduce

@rohanKanojia
Copy link
Member

@chirlo : Your point seems valid to me. Feel free to create a PR to move this discussion forward

@nbn
Copy link

nbn commented May 6, 2020

@rohanKanojia Great. Let me know if you need anything from me. I'll be happy to help you.

@longwa
Copy link

longwa commented Jul 10, 2020

Any progress on this?

I'm having the same issue applying a YAML file with a PVC defined.

client.load(inputStream).inNamespace(namespace).createOrReplace()

When trying to apply for the second time.

Failure executing: PUT at: https://x.x.x.x/api/v1/namespaces/k8s-test/persistentvolumeclaims/my-pvc. Message: PersistentVolumeClaim "my-pvc" is invalid: spec: Forbidden: is immutable after creation except resources.requests for bound claims. Received status: Status(apiVersion=v1, code=422, details=StatusDetails(causes=[StatusCause(field=spec, message=Forbidden: is immutable after creation except resources.requests for bound claims, reason=FieldValueForbidden, additionalProperties={})], group=null, kind=PersistentVolumeClaim, name=my-pvc, retryAfterSeconds=null, uid=null, additionalProperties={}), kind=Status, message=PersistentVolumeClaim "my-pvc" is invalid: spec: Forbidden: is immutable after creation except resources.requests for bound claims, metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=Invalid, status=Failure, additionalProperties={}).

If I take the same file and run kubectl apply -f myfile.yml --namespace foo it works fine.

Since createOrReplace has replaced the deprecated apply function, it seems like it should follow the same behavior.

@manusa
Copy link
Member

manusa commented Jul 15, 2020

Related to #2292

@rohanKanojia
Copy link
Member

@longwa : Hi, I'm not able to see the same behavior of kubectl in case of PVCs. Suppose I have this PVC:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: my-pvc
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 20Gi

When I create if for the first time, it gets created( I checked with fabric8-client SDK too, it gets created):

~/work/k8-resource-yamls : $ kubectl create -f ~/work/k8-resource-yamls/test-pvc.yml 
persistentvolumeclaim/my-pvc created

However, when I try to do kubectl apply -f again, it gives me error:

~/work/k8-resource-yamls : $ kubectl create -f ~/work/k8-resource-yamls/test-pvc.yml 
Error from server (AlreadyExists): error when creating "/home/rohaan/work/k8-resource-yamls/test-pvc.yml": persistentvolumeclaims "my-pvc" already exists

I think we should drop this ResourceHandler.equals() method which doesn't seem to be comparing objects properly, we should probably leave it to Kubernetes to calculate whether the object has changed or not.

@rohanKanojia
Copy link
Member

oh, okay. It's kubectl apply not kubectl create. Sorry for noise.

@longwa
Copy link

longwa commented Jul 27, 2020

Hi yes, it’s with apply. My understanding is that the createOrReplace should work like apply. IMO it was more clear with the now deprecated ‘apply’ function as that indicated a direct analog with k8s.

@rohanKanojia
Copy link
Member

rohanKanojia commented Jul 31, 2020

I think this issue should be closed by #2372

I wrote a test for this case:

@Test
void testCreateOrReplaceIgnoreWhenNoChange() {
// Given
PersistentVolumeClaim pvcOrignal = new PersistentVolumeClaimBuilder()
.withNewMetadata().withName("my-pvc").endMetadata()
.withNewSpec()
.withAccessModes("ReadWriteOnce")
.withNewResources()
.withRequests(Collections.singletonMap("storage", new Quantity("20Gi")))
.endResources()
.endSpec()
.build();
PersistentVolumeClaim pvcFromServer = new PersistentVolumeClaimBuilder()
.withNewMetadata()
.withName("my-pvc")
.withCreationTimestamp("2020-07-27T11:02:15Z")
.addToFinalizers("kubernetes.io/pvc-protection")
.withNamespace("default")
.withResourceVersion("203697")
.withSelfLink("/api/v1/namespaces/default/persistentvolumeclaims/my-pvc")
.withUid("60817eaa-19d8-41ba-adb4-3ea75860e1f8")
.endMetadata()
.withNewSpec()
.withAccessModes("ReadWriteOnce")
.withNewResources()
.withRequests(Collections.singletonMap("storage", new Quantity("20Gi")))
.endResources()
.withStorageClassName("standard")
.withVolumeMode("Filesystem")
.withVolumeName("pvc-60817eaa-19d8-41ba-adb4-3ea75860e1f8")
.endSpec()
.build();
server.expect().post().withPath("/api/v1/namespaces/ns1/persistentvolumeclaims")
.andReturn(HttpURLConnection.HTTP_CONFLICT, pvcFromServer).once();
server.expect().get().withPath("/api/v1/namespaces/ns1/persistentvolumeclaims/my-pvc")
.andReturn(HttpURLConnection.HTTP_OK, pvcFromServer).once();
KubernetesClient client = server.getClient();
// When
PersistentVolumeClaim pvcResult = client.persistentVolumeClaims().inNamespace("ns1").createOrReplace(pvcOrignal);
// Then
assertNotNull(pvcResult);
}
}

@rohanKanojia
Copy link
Member

I'm closing this, for now. But feel free to reopen in case it's not working as expected.

@chongyangxue
Copy link

Does this issue solved in latest version of kubernetes-client?
I'm having the same issue in 4.11.1.
@rohanKanojia

@rohanKanojia
Copy link
Member

rohanKanojia commented Sep 10, 2020

We had reverted it's fix due to this regression #2445 . Now we're discussing strategies on how to achieve this in #2454 . Please share your thoughts on #2454

@rohanKanojia rohanKanojia reopened this Sep 10, 2020
@manusa manusa changed the title creatOrReplace() should ignore if idential to last submitted: creatOrReplace() should ignore if identical to last submitted: Sep 10, 2020
@stale
Copy link

stale bot commented Dec 18, 2020

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@shawkins
Copy link
Contributor

Closing this issue. We'll let this be handled by the other more recent apply and server side apply issues #3896 #3334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants