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

Replacing ingress with createOrReplace ignores changes to annotations #2312

Closed
BartXZX opened this issue Jun 25, 2020 · 7 comments
Closed

Replacing ingress with createOrReplace ignores changes to annotations #2312

BartXZX opened this issue Jun 25, 2020 · 7 comments

Comments

@BartXZX
Copy link

BartXZX commented Jun 25, 2020

Hi guys,

I am replacing Kubernetes resources with client.resourceList(yaml).inNamespace(namespace).createOrReplace(), but my ingresses do not get replaced when only the annotations have changed. I use this for example to set/unset the nginx.ingress.kubernetes.io/whitelist-source-range annotation.
It looks like ResourceCompare#equals compares only labels, and not annotations.

Is this intentional, and do I need to work around this, or is this a bug?

@BartXZX BartXZX changed the title Replacing with createOrReplace ignores changes to annotations Replacing ingress with createOrReplace ignores changes to annotations Jun 25, 2020
@rohanKanojia
Copy link
Member

Related to #2292

@BartXZX
Copy link
Author

BartXZX commented Jun 26, 2020

@rohanKanojia Do you expect the related ticket to fix this issue?

@rohanKanojia
Copy link
Member

Umm, yeah. We're planning to refactor behavior of createOrReplace to do create() first and in case it fails with 409 do a replace
I think right now we're checking whether object has changed or not ourselves which is not aligning with kubectl behavior in my opinion.

@BartXZX
Copy link
Author

BartXZX commented Jun 26, 2020

Great! I've worked around it for now with a delete and apply, but this is less than ideal in case DNS decides to propagate the delete and not the create due to the timing of the ingress creation. Do you have an idea of when #2292 might be picked up?

@rohanKanojia
Copy link
Member

It's already in our Sprint backlog: https://github.com/eclipse/jkube/projects/6 . We would try it pick it whenever we can.

@rohanKanojia
Copy link
Member

I think this issue should be fixed via #2372 , I've added a test for this case here:

void testCreateOrReplaceWhenAnnotationUpdated() {
// Given
Ingress ingressFromServer = new IngressBuilder().withNewMetadata().withName("ing1").endMetadata().build();
Ingress ingressUpdated = new IngressBuilder(ingressFromServer).editOrNewMetadata()
.addToAnnotations("nginx.ingress.kubernetes.io/rewrite-target", "/")
.endMetadata().build();
server.expect().post().withPath("/apis/networking.k8s.io/v1beta1/namespaces/ns1/ingresses")
.andReturn(HttpURLConnection.HTTP_CONFLICT, ingressFromServer).once();
server.expect().get().withPath("/apis/networking.k8s.io/v1beta1/namespaces/ns1/ingresses/ing1")
.andReturn(HttpURLConnection.HTTP_OK, ingressFromServer).times(2);
server.expect().put().withPath("/apis/networking.k8s.io/v1beta1/namespaces/ns1/ingresses/ing1")
.andReturn(HttpURLConnection.HTTP_OK, ingressUpdated).once();
KubernetesClient client = server.getClient();
// When
ingressUpdated = client.network().ingresses().inNamespace("ns1").createOrReplace(ingressUpdated);
// Then
assertNotNull(ingressUpdated);
assertNotNull(ingressUpdated.getMetadata());
assertTrue(ingressUpdated.getMetadata().getAnnotations().containsKey("nginx.ingress.kubernetes.io/rewrite-target"));
}
}

@rohanKanojia
Copy link
Member

Closing this issue. But feel free to reopen if it doesn't work for you or you face any other problems.

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

No branches or pull requests

2 participants