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(k8s): correctly resolve manifests when build is set #4846

Merged
merged 30 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0d35d84
refactor: extract named function for manifest post-processing
vvagaytsev Sep 5, 2023
9430dcc
refactor: split manifest reading and post-processing
vvagaytsev Sep 5, 2023
649eddc
fix(k8s): correctly resolve manifests when `build` is set
edvald Jul 17, 2023
48abf35
fix(k8s): quickfix to avoid resetting `outdated` state to `ready`
vvagaytsev Jul 18, 2023
bc762e9
test: fix tests
vvagaytsev Aug 14, 2023
53dcb31
refactor(test): move helper to the upper-level scope
vvagaytsev Sep 6, 2023
2f192f2
refactor(test): introduce one more convenience helper
vvagaytsev Sep 6, 2023
00e33bf
refactor(test): introduce and rename some local vars
vvagaytsev Sep 6, 2023
120874b
refactor(test): use helpers to avoid code duplication
vvagaytsev Sep 6, 2023
b6f146f
chore: re-arranged code
vvagaytsev Sep 6, 2023
016fc43
chore(test): rename some tests and variables
vvagaytsev Sep 5, 2023
15b28a2
refactor(test): helper function to deploy in namespaces
vvagaytsev Sep 5, 2023
f3ec738
refactor(test): convert lambdas to functions
vvagaytsev Sep 5, 2023
485e05b
test: ensure all resources are deleted by `deleteKubernetesDeploy`
vvagaytsev Sep 6, 2023
dc3012d
test: restore initial module config state after each test
vvagaytsev Sep 6, 2023
0b82205
refactor(k8s): helper to compose k8s deploy status
vvagaytsev Sep 6, 2023
a3d44f5
refactor(k8s): return deploy status immediately on "missing" state
vvagaytsev Sep 7, 2023
0f60396
refactor: unwrap unnecessary else-block
vvagaytsev Sep 7, 2023
7f898d5
chore: variable scoping
vvagaytsev Sep 7, 2023
da6a0b8
refactor: extract helper to check if deploy is outdated
vvagaytsev Sep 7, 2023
6b60f03
refactor: move input args check into `getForwardablePorts`
vvagaytsev Sep 7, 2023
3ef9725
chore: remove unnecessary code and comments
vvagaytsev Sep 7, 2023
f702c7f
chore: remove unnecessary try/catch
vvagaytsev Sep 7, 2023
8bab6c2
chore: remove unused local var
vvagaytsev Sep 7, 2023
297f5cd
refactor: use SRP in status calculation
vvagaytsev Sep 7, 2023
3a1b913
refactor: simplified code
vvagaytsev Sep 7, 2023
08b7a37
test: fix test setup to cover the original bug
vvagaytsev Sep 7, 2023
30f1435
chore: post-merge corrections
vvagaytsev Sep 19, 2023
60eee4c
refactor: remove unnecessary function call
vvagaytsev Sep 20, 2023
07c5427
chore: typos and wording
vvagaytsev Sep 20, 2023
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
5 changes: 2 additions & 3 deletions core/src/plugins/kubernetes/helm/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const helmDeploy: DeployActionHandler<"deploy", HelmDeployAction> = async
await helm({ ctx: k8sCtx, namespace, log, args: [...upgradeArgs], emitLogEvents: true })

// If ctx.cloudApi is defined, the user is logged in and they might be trying to deploy to an environment
// that could have been paused by by Garden Cloud's AEC functionality. We therefore make sure to clean up any
// that could have been paused by Garden Cloud's AEC functionality. We therefore make sure to clean up any
// dangling annotations created by Garden Cloud.
if (ctx.cloudApi) {
try {
Expand All @@ -92,7 +92,7 @@ export const helmDeploy: DeployActionHandler<"deploy", HelmDeployAction> = async
})
)
} catch (error) {
const errorMsg = `Failed to remove Garden Cloud AEC annotations for service: ${action.name}.`
const errorMsg = `Failed to remove Garden Cloud AEC annotations for deploy: ${action.name}.`
log.warn(errorMsg)
log.debug({ error: toGardenError(error) })
}
Expand Down Expand Up @@ -148,7 +148,6 @@ export const helmDeploy: DeployActionHandler<"deploy", HelmDeployAction> = async
timeoutSec: timeout,
})

// Local mode has its own port-forwarding configuration
const forwardablePorts = getForwardablePorts({ resources: manifests, parentAction: action, mode })

// Make sure port forwards work after redeployment
Expand Down
175 changes: 122 additions & 53 deletions core/src/plugins/kubernetes/kubernetes-type/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

import { join, resolve } from "path"
import { pathExists, readFile } from "fs-extra"
import { flatten, set } from "lodash"
import { flatten, keyBy, set } from "lodash"
import { loadAll } from "js-yaml"

import { KubernetesModule } from "./module-config"
import { KubernetesResource } from "../types"
import { KubeApi } from "../api"
import { dedent, gardenAnnotationKey, naturalList } from "../../../util/string"
import { dedent, gardenAnnotationKey, naturalList, stableStringify } from "../../../util/string"
import { Log } from "../../../logger/log-entry"
import { PluginContext } from "../../../plugin-context"
import { ConfigurationError, GardenError, PluginError } from "../../../exceptions"
Expand All @@ -23,9 +23,10 @@ import { HelmModule } from "../helm/module-config"
import { KubernetesDeployAction } from "./config"
import { CommonRunParams } from "../../../plugin/handlers/Run/run"
import { runAndCopy } from "../run"
import { getResourceContainer, getResourcePodSpec, getTargetResource, makePodName } from "../util"
import { Resolved } from "../../../actions/types"
import { getResourceContainer, getResourceKey, getResourcePodSpec, getTargetResource, makePodName } from "../util"
import { ActionMode, Resolved } from "../../../actions/types"
import { KubernetesPodRunAction, KubernetesPodTestAction } from "./kubernetes-pod"
import { V1ConfigMap } from "@kubernetes/client-node"
import { glob } from "glob"
import isGlob from "is-glob"
import pFilter from "p-filter"
Expand Down Expand Up @@ -55,54 +56,65 @@ export async function getManifests({
log,
action,
defaultNamespace,
readFromSrcDir = false,
}: {
ctx: PluginContext
api: KubeApi
log: Log
action: Resolved<KubernetesDeployAction | KubernetesPodRunAction | KubernetesPodTestAction>
defaultNamespace: string
readFromSrcDir?: boolean
}): Promise<KubernetesResource[]> {
const declaredManifests: DeclaredManifest[] = await Promise.all(
(await readManifests(ctx, action, log, readFromSrcDir)).map(async ({ manifest, declaration }) => {
// Ensure a namespace is set, if not already set, and if required by the resource type
if (!manifest.metadata?.namespace) {
if (!manifest.metadata) {
// TODO: Type system complains that name is missing
;(manifest as any).metadata = {}
}

const info = await api.getApiResourceInfo(log, manifest.apiVersion, manifest.kind)

if (info?.namespaced) {
manifest.metadata.namespace = defaultNamespace
}
// Local function to set some default values and Garden-specific annotations.
async function postProcessManifest({ manifest, declaration }: DeclaredManifest): Promise<DeclaredManifest> {
// Ensure a namespace is set, if not already set, and if required by the resource type
if (!manifest.metadata?.namespace) {
if (!manifest.metadata) {
// TODO: Type system complains that name is missing
;(manifest as any).metadata = {}
}

/**
* Set Garden annotations.
*
* For namespace resources, we use the namespace's name as the annotation value, to ensure that namespace resources
* with different names aren't considered by Garden to be the same resource.
*
* This is relevant e.g. in the context of a shared dev cluster, where several users might create their own
* copies of a namespace resource (each named e.g. "${username}-some-namespace") through deploying a `kubernetes`
* module that includes a namespace resource in its manifests.
*/
const annotationValue =
manifest.kind === "Namespace" ? gardenNamespaceAnnotationValue(manifest.metadata.name) : action.name
set(manifest, ["metadata", "annotations", gardenAnnotationKey("service")], annotationValue)
set(manifest, ["metadata", "annotations", gardenAnnotationKey("mode")], action.mode())
set(manifest, ["metadata", "labels", gardenAnnotationKey("service")], annotationValue)

return { manifest, declaration }
})
)
const info = await api.getApiResourceInfo(log, manifest.apiVersion, manifest.kind)

if (info?.namespaced) {
manifest.metadata.namespace = defaultNamespace
}
}

validateDeclaredManifests(declaredManifests)
/**
* Set Garden annotations.
*
* For namespace resources, we use the namespace's name as the annotation value, to ensure that namespace resources
* with different names aren't considered by Garden to be the same resource.
*
* This is relevant e.g. in the context of a shared dev cluster, where several users might create their own
* copies of a namespace resource (each named e.g. "${username}-some-namespace") through deploying a `kubernetes`
* module that includes a namespace resource in its manifests.
*/
const annotationValue =
manifest.kind === "Namespace" ? gardenNamespaceAnnotationValue(manifest.metadata.name) : action.name
set(manifest, ["metadata", "annotations", gardenAnnotationKey("service")], annotationValue)
set(manifest, ["metadata", "annotations", gardenAnnotationKey("mode")], action.mode())
set(manifest, ["metadata", "labels", gardenAnnotationKey("service")], annotationValue)

return { manifest, declaration }
}

const declaredManifests = await readManifests(ctx, action, log)

if (action.kind === "Deploy") {
// Add metadata ConfigMap to aid quick status check
const metadataManifest = getMetadataManifest(action, defaultNamespace, declaredManifests)
const declaredMetadataManifest: DeclaredManifest = {
declaration: { type: "inline", index: declaredManifests.length },
manifest: metadataManifest,
}
declaredManifests.push(declaredMetadataManifest)
}

return declaredManifests.map((m) => m.manifest)
const postProcessedManifests: DeclaredManifest[] = await Promise.all(declaredManifests.map(postProcessManifest))

validateDeclaredManifests(postProcessedManifests)

return postProcessedManifests.map((m) => m.manifest)
}

/**
Expand Down Expand Up @@ -156,24 +168,81 @@ export function validateDeclaredManifests(declaredManifests: DeclaredManifest[])
}
}

export interface ManifestMetadata {
key: string
apiVersion: string
kind: string
name: string
namespace: string
}

export interface ParsedMetadataManifestData {
resolvedVersion: string
mode: ActionMode
manifestMetadata: { [key: string]: ManifestMetadata }
}

export function getMetadataManifest(
action: Resolved<KubernetesDeployAction>,
defaultNamespace: string,
declaredManifests: DeclaredManifest[]
): KubernetesResource<V1ConfigMap> {
const manifestMetadata: ManifestMetadata[] = declaredManifests.map((declaredManifest) => {
const m = declaredManifest.manifest
return {
key: getResourceKey(m),
apiVersion: m.apiVersion,
kind: m.kind,
name: m.metadata.name,
namespace: m.metadata.namespace || defaultNamespace,
}
})

return {
apiVersion: "v1",
kind: "ConfigMap",
metadata: {
name: `garden-meta-${action.kind.toLowerCase()}-${action.name}`,
},
data: {
resolvedVersion: action.versionString(),
mode: action.mode(),
manifestMetadata: stableStringify(keyBy(manifestMetadata, "key")),
},
}
}

export function parseMetadataResource(log: Log, resource: KubernetesResource<V1ConfigMap>): ParsedMetadataManifestData {
// TODO: validate schema here
const output: ParsedMetadataManifestData = {
resolvedVersion: resource.data?.resolvedVersion || "",
mode: (resource.data?.mode || "default") as ActionMode,
manifestMetadata: {},
}

const manifestMetadata = resource.data?.manifestMetadata

if (manifestMetadata) {
try {
// TODO: validate by schema
output.manifestMetadata = JSON.parse(manifestMetadata)
} catch (error) {
log.debug({ msg: `Failed querying for remote resources: ${error}` })
}
}

return output
}

/**
* Read the manifests from the module config, as well as any referenced files in the config.
*
* @param module The kubernetes module to read manifests for.
* @param readFromSrcDir Whether or not to read the manifests from the module build dir or from the module source dir.
* In general we want to read from the build dir to ensure that manifests added via the `build.dependencies[].copy`
* field will be included. However, in some cases, e.g. when getting the service status, we can't be certain that
* the build has been staged and we therefore read the manifests from the source.
*
* TODO: Remove this once we're checking for kubernetes module service statuses with version hashes.
*/
async function readManifests(
export async function readManifests(
ctx: PluginContext,
action: Resolved<KubernetesDeployAction | KubernetesPodRunAction | KubernetesPodTestAction>,
log: Log,
readFromSrcDir = false
log: Log
): Promise<DeclaredManifest[]> {
const manifestPath = readFromSrcDir ? action.basePath() : action.getBuildPath()
const manifestPath = action.getBuildPath()

const inlineManifests = readInlineManifests(action)
const fileManifests = await readFileManifests(ctx, action, log, manifestPath)
Expand Down
Loading