fix(k8s): handle CronJob resources correctly

This would also have applied in cases where a group had different resource types under different API versions. Which is not particularly common, but most likely affected some other resource types. This also slightly reduces the number of API calls needed on init. Fixes #1275
garden-io · Oct 31, 2019 · e7a4646 · e7a4646
1 parent 1c760c0
commit e7a4646
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 32 deletions.
diff --git a/garden-service/src/plugins/kubernetes/api.ts b/garden-service/src/plugins/kubernetes/api.ts
@@ -45,23 +45,18 @@ interface ApiGroupMap {
 }
 
 interface ApiResourceMap {
-  [kind: string]: V1APIResource
+  [group: string]: { [kind: string]: V1APIResource }
 }
 
 interface ApiInfo {
   coreApi: V1APIVersions
   groups: V1APIGroup[]
   groupMap: ApiGroupMap
-  resources: { [group: string]: ApiResourceMap }
-}
-
-interface ApiResourceInfo {
-  group: V1APIGroup
-  resource: V1APIResource
 }
 
 const cachedConfigs: { [context: string]: KubeConfig } = {}
 const cachedApiInfo: { [context: string]: ApiInfo } = {}
+const cachedApiResourceInfo: { [context: string]: ApiResourceMap } = {}
 const apiInfoLock = new AsyncLock()
 
 // NOTE: be warned, the API of the client library is very likely to change
@@ -204,7 +199,6 @@ export class KubeApi {
           coreApi,
           groups,
           groupMap,
-          resources: {},
         }
 
         cachedApiInfo[this.context] = info
@@ -229,25 +223,29 @@ export class KubeApi {
     return group
   }
 
-  async getApiResourceInfo(log: LogEntry, manifest: KubernetesResource): Promise<ApiResourceInfo> {
-    const apiInfo = await this.getApiInfo()
-    const group = await this.getApiGroup(manifest)
-    const groupId = group.preferredVersion!.groupVersion
+  async getApiResourceInfo(log: LogEntry, manifest: KubernetesResource): Promise<V1APIResource> {
+    const apiVersion = manifest.apiVersion
+
+    if (!cachedApiResourceInfo[this.context]) {
+      cachedApiResourceInfo[this.context] = {}
+    }
+
+    const apiResources = cachedApiResourceInfo[this.context]
 
-    const lockKey = `${this.context}/${groupId}`
-    const resourceMap = apiInfo.resources[groupId] || await apiInfoLock.acquire(lockKey, async () => {
-      if (apiInfo.resources[groupId]) {
-        return apiInfo.resources[groupId]
+    const lockKey = `${this.context}/${apiVersion}`
+    const resourceMap = apiResources[apiVersion] || await apiInfoLock.acquire(lockKey, async () => {
+      if (apiResources[apiVersion]) {
+        return apiResources[apiVersion]
       }
 
-      log.debug(`Kubernetes: Getting API resource info for group ${groupId}`)
-      const res = await this.request(log, getGroupBasePath(groupId))
+      log.debug(`Kubernetes: Getting API resource info for group ${apiVersion}`)
+      const res = await this.request(log, getGroupBasePath(apiVersion))
 
       // We're only interested in the entities themselves, not the sub-resources
-      const resources = res.body.resources.filter(r => !r.name.includes("/"))
+      const resources = res.body.resources.filter((r: any) => !r.name.includes("/"))
 
-      apiInfo.resources[groupId] = keyBy(resources, "kind")
-      return apiInfo.resources[groupId]
+      apiResources[apiVersion] = keyBy(resources, "kind")
+      return apiResources[apiVersion]
     })
 
     const resource = resourceMap[manifest.kind]
@@ -258,7 +256,7 @@ export class KubeApi {
       })
     }
 
-    return { group, resource }
+    return resource
   }
 
   async request(log: LogEntry, path: string, opts: Omit<request.OptionsWithUrl, "url"> = {}): Promise<any> {
@@ -289,13 +287,13 @@ export class KubeApi {
     const name = manifest.metadata.name
     log.silly(`Fetching Kubernetes resource ${manifest.apiVersion}/${manifest.kind}/${name}`)
 
-    const { group, resource } = await this.getApiResourceInfo(log, manifest)
-    const groupId = group.preferredVersion!.groupVersion
-    const basePath = getGroupBasePath(groupId)
+    const resourceInfo = await this.getApiResourceInfo(log, manifest)
+    const apiVersion = manifest.apiVersion
+    const basePath = getGroupBasePath(apiVersion)
 
-    const apiPath = resource.namespaced
-      ? `${basePath}/namespaces/${namespace}/${resource.name}/${name}`
-      : `${basePath}/${resource.name}/${name}`
+    const apiPath = resourceInfo.namespaced
+      ? `${basePath}/namespaces/${namespace}/${resourceInfo.name}/${name}`
+      : `${basePath}/${resourceInfo.name}/${name}`
 
     const res = await this.request(log, apiPath)
     return res.body
@@ -340,7 +338,7 @@ export class KubeApi {
           return Reflect.get(target, name, receiver)
         }
 
-        return function(...args) {
+        return (...args: any[]) => {
           const defaultHeaders = target["defaultHeaders"]
 
           if (name.startsWith("patch")) {
@@ -372,9 +370,9 @@ export class KubeApi {
   }
 }
 
-function getGroupBasePath(groupId: string) {
+function getGroupBasePath(apiVersion: string) {
   // Of course, Kubernetes helpfully uses a singular for the core API and not everything else. So there you go.
-  return groupId.includes("/") ? `/apis/${groupId}` : `/api/${groupId}`
+  return apiVersion.includes("/") ? `/apis/${apiVersion}` : `/api/${apiVersion}`
 }
 
 export async function getKubeConfig(log: LogEntry, provider: KubernetesProvider) {

diff --git a/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts b/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts
@@ -170,7 +170,7 @@ async function getManifests(
     if (!manifest.metadata.namespace) {
       const info = await api.getApiResourceInfo(log, manifest)
 
-      if (info.resource.namespaced) {
+      if (info.namespaced) {
         manifest.metadata.namespace = defaultNamespace
       }
     }