Collaborne / auto-kubernetes-client

NodeJS Kubernetes Client with automatic API discovery
Apache License 2.0
8 stars 3 forks source link

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

Open arobson opened 5 years ago

arobson commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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.