Skip to content
This repository has been archived by the owner on Apr 19, 2021. It is now read-only.

Enable auto-kubernetes-client to report API group errors and continu #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Enable auto-kubernetes-client to report API group errors and continu #9

wants to merge 1 commit into from

Conversation

arobson
Copy link

@arobson arobson commented Jan 5, 2019

Presently there are conditions that can exist within a Kubernetes cluster that may cause certain API groups to fail to respond to requests for API metadata during the k8sRequest to the groupPath inside the createApi call.

If this should happen, it becomes impossible to communicate with the cluster at all. In our use case, where we our building Kubernetes tooling, we can't perform any communication with the cluster at all even across healthy aspects of the functioning API groups because some unrelated API group might be malfunctioning (example: 'metrics.k8s.io' from 'apis/metrics.k8s.io/v1beta1').

This change would make it so that a console error is reported but then the rest of the available API groups are created so that the available groups can function. It would still be the responsibility of an upstream library to handle errors/rejected promises in the event of missing groups but the alternative is being 100% dead in the water until you have a completely functioning cluster, which doesn't make a lot of sense for operational tooling.

I hope this makes sense. Happy to discuss other approaches, but would love to avoid maintaining a fork. Thank you very much for this library, it's the best way I've seen to interact with the Kubernetes API.

@ankon
Copy link
Contributor

ankon commented Jan 7, 2019

I think this definitely makes sense.

Only thing I'm pondering about: Instead of behaving as if the API didn't exist, would it maybe make more sense to produce some form of "always-fails" API implementation, and/or track that we saw the API, but were unable to consume it?
Right now I'm relying on this crashing behavior in our use case, but arguably that is wrong, and if I could validate instead that everything was found I should move that crashing into our own logic :)

@ankon
Copy link
Contributor

ankon commented Jan 7, 2019

On top of your changes, something like this:

diff --git a/src/index.js b/src/index.js
index b2a9959..aa8511d 100644
--- a/src/index.js
+++ b/src/index.js
@@ -361,12 +361,7 @@ function connect(config) {
 		}
 
 		return k8sRequest(groupPath, {})
-			.then(
-				createApiFromResources,
-				() => {
-					console.error(`auto-kubernetes-client failed to acquire API definition for '${apiName}' from '${groupPath}'`);
-				}
-			);
+			.then(createApiFromResources);
 	}
 
 	const coreVersion = config.version || 'v1';
@@ -374,7 +369,20 @@ function connect(config) {
 	return k8sRequest('apis').then(apiGroups => {
 		// Initialize the APIs
 		const apiPromises = flatMap(apiGroups.groups, group => {
-			return group.versions.map(version => createApi(group.name, `apis/${version.groupVersion}`, version, version.version === group.preferredVersion.version));
+			return group.versions.map(version => {
+				const apiName = group.name;
+				const groupPath = `apis/${version.groupVersion}`;
+				const isPreferred = version.version === group.preferredVersion.version;
+				return createApi(apiName, groupPath, version, isPreferred).catch(e => {
+					// XXX: Unify with createApi... above
+					return {
+						error: e.message,
+						name: apiName,
+						preferred: isPreferred,
+						version: version.version,
+					};
+				});
+			});
 		});
 		apiPromises.push(createApi('', `api/${coreVersion}`, {
 			groupVersion: coreVersion,
@@ -384,12 +392,13 @@ function connect(config) {
 	}).then(apis => {
 		return apis.reduce((result, api) => {
 			// Build a compatible name for this API. Note that both api.name and api.version can end up empty here.
-			if (api) {
-				const apiNamePrefix = api.name ? `${api.name}/` : '';
-				result[`${apiNamePrefix}${api.version}`] = api;
-				if (api.preferred) {
-					result[api.name] = api;
-				}
+			const apiNamePrefix = api.name ? `${api.name}/` : '';
+			result[`${apiNamePrefix}${api.version}`] = api;
+			if (api.preferred) {
+				result[api.name] = api;
+			}
+			if (api.error) {
+				console.error(`Failed to acquire API ${api.name}: ${api.error}`);
 			}
 			return result;
 		}, {});

@arobson
Copy link
Author

arobson commented Jan 7, 2019

@ankon - ah, returning a proxy that always fails is a great idea 👍I would happily use that, it's just that we can't have a situation where the entire system fails and we can't use the available groups.

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

Successfully merging this pull request may close these issues.

None yet

2 participants