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

fix: discovery fail when at least one api is unavailable #2732

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,24 @@
import io.kubernetes.client.Discovery;
import io.kubernetes.client.extended.kubectl.exception.KubectlException;
import io.kubernetes.client.openapi.ApiException;
import io.kubernetes.client.util.exception.IncompleteDiscoveryException;
import java.util.Set;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class KubectlApiResources extends Kubectl.ApiClientBuilder<KubectlApiResources>
implements Kubectl.Executable<Set<Discovery.APIResource>> {

private static final Logger LOGGER = LoggerFactory.getLogger(KubectlApiResources.class);

@Override
public Set<Discovery.APIResource> execute() throws KubectlException {
Discovery discovery = new Discovery(this.apiClient);
try {
return discovery.findAll();
} catch (IncompleteDiscoveryException e) {
LOGGER.warn("Error while getting all api resources, some resources will be missing", e);
return e.getDiscoveredResources();
} catch (ApiException e) {
throw new KubectlException(e);
}
Expand Down
27 changes: 22 additions & 5 deletions util/src/main/java/io/kubernetes/client/Discovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.kubernetes.client.openapi.models.V1APIGroupList;
import io.kubernetes.client.openapi.models.V1APIResourceList;
import io.kubernetes.client.openapi.models.V1APIVersions;
import io.kubernetes.client.util.exception.IncompleteDiscoveryException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -50,12 +51,28 @@ public Set<APIResource> findAll() throws ApiException {
for (String version : legacyCoreApi().getVersions()) {
allResources.addAll(findAll("", Arrays.asList(version), version, "/api/" + version));
}
IncompleteDiscoveryException incompleteDiscoveryException = null;
for (V1APIGroup group : groupDiscovery("/apis").getGroups()) {
allResources.addAll(
findAll(
group.getName(),
group.getVersions().stream().map(v -> v.getVersion()).collect(Collectors.toList()),
group.getPreferredVersion().getVersion()));
try {
allResources.addAll(
findAll(
group.getName(),
group.getVersions().stream().map(v -> v.getVersion()).collect(Collectors.toList()),
group.getPreferredVersion().getVersion()));
} catch (ApiException e){
IncompleteDiscoveryException resourceDiscoveryException = new IncompleteDiscoveryException(
String.format("Unable to retrieve the complete list of server APIs: %s/%s : %s",
group.getName(), group.getPreferredVersion().getVersion(), e.getResponseBody()),
e, allResources);
if (incompleteDiscoveryException == null) {
incompleteDiscoveryException = resourceDiscoveryException;
}else {
incompleteDiscoveryException.addSuppressed(resourceDiscoveryException);
}
}
}
if (incompleteDiscoveryException != null) {
throw incompleteDiscoveryException;
}
return allResources;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io.kubernetes.client.common.KubernetesListObject;
import io.kubernetes.client.common.KubernetesObject;
import io.kubernetes.client.openapi.ApiException;
import io.kubernetes.client.util.exception.IncompleteDiscoveryException;
import java.io.File;

import java.io.IOException;
Expand Down Expand Up @@ -266,7 +267,13 @@ public static Set<Discovery.APIResource> refresh(Discovery discovery, Duration r
return lastAPIDiscovery;
}

Set<Discovery.APIResource> apiResources = discovery.findAll();
Set<Discovery.APIResource> apiResources = null;
try {
apiResources = discovery.findAll();
} catch (IncompleteDiscoveryException e) {
logger.warn("Error while getting all api resources, some api resources will not be refreshed", e);
apiResources = e.getDiscoveredResources();
}

for (Discovery.APIResource apiResource : apiResources) {
for (String version : apiResource.getVersions()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package io.kubernetes.client.util.exception;

import io.kubernetes.client.Discovery.APIResource;
import io.kubernetes.client.openapi.ApiException;
import java.util.Set;

public class IncompleteDiscoveryException extends ApiException {

private static final long serialVersionUID = 1L;

private final transient Set<APIResource> discoveredResources;

public IncompleteDiscoveryException(String message, ApiException cause, Set<APIResource> discoveredResources) {
super(message, cause, cause.getCode(), cause.getResponseHeaders(), cause.getResponseBody());
this.discoveredResources = discoveredResources;
}

public Set<APIResource> getDiscoveredResources() {
return discoveredResources;
}

}
147 changes: 147 additions & 0 deletions util/src/test/java/io/kubernetes/client/DiscoveryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,24 @@
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

import com.github.tomakehurst.wiremock.junit.WireMockRule;
import io.kubernetes.client.Discovery.APIResource;
import io.kubernetes.client.openapi.ApiClient;
import io.kubernetes.client.openapi.ApiException;
import io.kubernetes.client.openapi.models.V1APIGroup;
import io.kubernetes.client.openapi.models.V1APIGroupList;
import io.kubernetes.client.openapi.models.V1APIResource;
import io.kubernetes.client.openapi.models.V1APIResourceList;
import io.kubernetes.client.openapi.models.V1APIVersions;
import io.kubernetes.client.openapi.models.V1GroupVersionForDiscovery;
import io.kubernetes.client.util.ClientBuilder;
import io.kubernetes.client.util.exception.IncompleteDiscoveryException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import org.junit.Before;
import org.junit.Rule;
Expand Down Expand Up @@ -101,4 +109,143 @@ public void testGroupResourcesByName() {
assertEquals(true, meow.getNamespaced());
assertEquals("mouse", meow.getSubResources().get(0));
}

@Test
public void findAllShouldReturnAllApiResourceWhenAllResourceDiscoveryApiResponseAreSuccess() throws ApiException {
Discovery discovery = new Discovery(apiClient);

wireMockRule.stubFor(
get("/api")
.willReturn(
aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(
apiClient
.getJSON()
.serialize(new V1APIVersions()))));

String group = "test.com";
String version = "v1";
String path="/apis/"+group+'/'+version;

wireMockRule.stubFor(
get("/apis")
.willReturn(
aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(
apiClient
.getJSON()
.serialize(new V1APIGroupList()
.addGroupsItem(new V1APIGroup()
.name(group)
.preferredVersion(new V1GroupVersionForDiscovery().version(version))
.versions(Arrays.asList(
new V1GroupVersionForDiscovery().version(version))))))));

wireMockRule.stubFor(
get(urlPathEqualTo(path))
.willReturn(
aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(
apiClient
.getJSON()
.serialize(new V1APIResourceList()
.resources(
Arrays.asList(
new V1APIResource()
.name("first"),
new V1APIResource()
.name("second")))))));

List<String> versions = new ArrayList<>();
versions.add(version);
Set<APIResource> apiResources = discovery.findAll();
wireMockRule.verify(1, getRequestedFor(urlPathEqualTo(path)));
assertEquals(2, apiResources.size());
}

@Test
public void findAllShouldThrowImcompleteDiscoveryExceptionWhenAtLeastOneResourceDiscoveryApiResponseIsNotSuccess() throws ApiException {
Discovery discovery = new Discovery(apiClient);

wireMockRule.stubFor(
get("/api")
.willReturn(
aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(
apiClient
.getJSON()
.serialize(new V1APIVersions()))));

String groupSuccess = "test.com";
String version = "v1";
String pathSuccess="/apis/"+groupSuccess+'/'+version;

String groupError = "testError.com";
String pathError="/apis/"+groupError+'/'+version;

wireMockRule.stubFor(
get("/apis")
.willReturn(
aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(
apiClient
.getJSON()
.serialize(new V1APIGroupList()
.addGroupsItem(new V1APIGroup()
.name(groupError)
.preferredVersion(new V1GroupVersionForDiscovery().version(version))
.versions(Arrays.asList(
new V1GroupVersionForDiscovery().version(version))))
.addGroupsItem(new V1APIGroup()
.name(groupSuccess)
.preferredVersion(new V1GroupVersionForDiscovery().version(version))
.versions(Arrays.asList(
new V1GroupVersionForDiscovery().version(version))))))));

wireMockRule.stubFor(
get(urlPathEqualTo(pathError))
.willReturn(
aResponse()
.withStatus(503)));

wireMockRule.stubFor(
get(urlPathEqualTo(pathSuccess))
.willReturn(
aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(
apiClient
.getJSON()
.serialize(new V1APIResourceList()
.resources(
Arrays.asList(
new V1APIResource()
.name("first"),
new V1APIResource()
.name("second")))))));

List<String> versions = new ArrayList<>();
versions.add(version);
Set<APIResource> apiResources = null;
try{
discovery.findAll();
fail("Should have throw ImcompleteDiscoveryException");
} catch (IncompleteDiscoveryException e) {
apiResources = e.getDiscoveredResources();
}
wireMockRule.verify(1, getRequestedFor(urlPathEqualTo(pathError)));
wireMockRule.verify(1, getRequestedFor(urlPathEqualTo(pathSuccess)));
assertEquals(2, apiResources.size());
}
}
Loading