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 #4516

Merged
merged 6 commits into from
Jul 7, 2023
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
2 changes: 1 addition & 1 deletion core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"execa": "^4.1.0",
"fs-extra": "^11.1.0",
"get-port": "^5.1.1",
"glob": "^7.2.3",
"glob": "^10.2.6",
"global-agent": "^3.0.0",
"got": "^11.8.6",
"gray-matter": "^4.0.3",
Expand Down
11 changes: 2 additions & 9 deletions core/src/config/template-contexts/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,8 @@ export class ActionSpecContext extends OutputConfigContext {
public this: ActionReferenceContext

constructor(params: ActionSpecContextParams) {
const {
action,
garden,
partialRuntimeResolution,
variables,
inputs,
resolvedDependencies,
executedDependencies,
} = params
const { action, garden, partialRuntimeResolution, variables, inputs, resolvedDependencies, executedDependencies } =
params

const internal = action.getInternal()

Expand Down
37 changes: 23 additions & 14 deletions core/src/plugins/kubernetes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ export interface ExecInPodResult {
timedOut: boolean
}

interface ReadParams {
log: Log
namespace: string
apiVersion: string
kind: string
name: string
}

export class KubeApi {
public apis: WrappedApi<ApisApi>
public apps: WrappedApi<AppsV1Api>
Expand Down Expand Up @@ -357,19 +365,7 @@ export class KubeApi {
/**
* Fetch the specified resource from the cluster.
*/
async read({
log,
namespace,
apiVersion,
kind,
name,
}: {
log: Log
namespace: string
apiVersion: string
kind: string
name: string
}) {
async read({ log, namespace, apiVersion, kind, name }: ReadParams) {
log.silly(`Fetching Kubernetes resource ${apiVersion}/${kind}/${name}`)

const typePath = await this.getResourceTypeApiPath({
Expand All @@ -385,6 +381,19 @@ export class KubeApi {
return res.body
}

async readOrNull(params: ReadParams) {
try {
const resource = await this.read(params)
return resource
} catch (err) {
if (err.statusCode === 404) {
return null
} else {
throw err
}
}
}

/**
* Given a manifest, attempt to read the matching resource from the cluster.
*/
Expand All @@ -400,7 +409,7 @@ export class KubeApi {
/**
* Same as readBySpec() but returns null if the resource is missing.
*/
async readOrNull(params: { log: Log; namespace: string; manifest: KubernetesResource }) {
async readBySpecOrNull(params: { log: Log; namespace: string; manifest: KubernetesResource }) {
try {
const resource = await this.readBySpec(params)
return resource
Expand Down
8 changes: 7 additions & 1 deletion core/src/plugins/kubernetes/container/build/buildkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,13 @@ export async function ensureBuildkit({

// Check status of the buildkit deployment
const manifest = getBuildkitDeployment(provider, authSecret.metadata.name, imagePullSecrets)
const status = await compareDeployedResources(ctx as KubernetesPluginContext, api, namespace, [manifest], deployLog)
const status = await compareDeployedResources({
ctx: ctx as KubernetesPluginContext,
api,
namespace,
manifests: [manifest],
log: deployLog,
})

if (status.state === "ready") {
// Need to wait a little to ensure the secret is updated in the deployment
Expand Down
12 changes: 6 additions & 6 deletions core/src/plugins/kubernetes/container/build/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,13 +314,13 @@ export async function ensureUtilDeployment({

// Check status of the util deployment
const { deployment, service } = getUtilManifests(provider, authSecret.metadata.name, imagePullSecrets)
const status = await compareDeployedResources(
ctx as KubernetesPluginContext,
const status = await compareDeployedResources({
ctx: ctx as KubernetesPluginContext,
api,
namespace,
[deployment, service],
deployLog
)
manifests: [deployment, service],
log: deployLog,
})

if (status.state === "ready") {
// Need to wait a little to ensure the secret is updated in the deployment
Expand Down Expand Up @@ -396,7 +396,7 @@ export async function ensureBuilderSecret({
const secretName = `garden-docker-auth-${hash}`
authSecret.metadata.name = secretName

const existingSecret = await api.readOrNull({ log, namespace, manifest: authSecret })
const existingSecret = await api.readBySpecOrNull({ log, namespace, manifest: authSecret })

if (!existingSecret || authSecret.data?.[dockerAuthSecretKey] !== existingSecret.data?.[dockerAuthSecretKey]) {
log.info(chalk.gray(`-> Updating Docker auth secret in namespace ${namespace}`))
Expand Down
3 changes: 1 addition & 2 deletions core/src/plugins/kubernetes/container/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,12 @@ export const k8sGetContainerDeployStatus: DeployActionHandler<"getStatus", Conta
action,
imageId,
})

let {
state,
remoteResources,
mode: deployedMode,
selectorChangedResourceKeys,
} = await compareDeployedResources(k8sCtx, api, namespace, manifests, log)
} = await compareDeployedResources({ ctx: k8sCtx, api, namespace, manifests, log })
const ingresses = await getIngresses(action, api, provider)

return prepareContainerDeployStatus({
Expand Down
8 changes: 7 additions & 1 deletion core/src/plugins/kubernetes/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,13 @@ export async function getEnvironmentStatus({

if (provider.config.buildMode !== "local-docker") {
const authSecret = await prepareDockerAuth(api, provider, systemNamespace)
const comparison = await compareDeployedResources(k8sCtx, api, systemNamespace, [authSecret], log)
const comparison = await compareDeployedResources({
ctx: k8sCtx,
api,
namespace: systemNamespace,
manifests: [authSecret],
log,
})
secretsUpToDate = comparison.state === "ready"
}

Expand Down
101 changes: 81 additions & 20 deletions core/src/plugins/kubernetes/kubernetes-type/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
import { resolve } from "path"
import { readFile } from "fs-extra"
import Bluebird from "bluebird"
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 { gardenAnnotationKey } from "../../../util/string"
import { gardenAnnotationKey, stableStringify } from "../../../util/string"
import { Log } from "../../../logger/log-entry"
import { PluginContext } from "../../../plugin-context"
import { ConfigurationError, PluginError } from "../../../exceptions"
Expand All @@ -24,9 +24,11 @@ import { HelmModule } from "../helm/module-config"
import { KubernetesDeployAction } from "./config"
import { CommonRunParams } from "../../../plugin/handlers/Run/run"
import { runAndCopy } from "../run"
import { getTargetResource, getResourcePodSpec, getResourceContainer, makePodName } from "../util"
import { Resolved } from "../../../actions/types"
import { getTargetResource, getResourcePodSpec, getResourceContainer, makePodName, getResourceKey } from "../util"
import { ActionMode, Resolved } from "../../../actions/types"
import { KubernetesPodRunAction, KubernetesPodTestAction } from "./kubernetes-pod"
import { V1ConfigMap } from "@kubernetes/client-node"
import { glob } from "glob"

/**
* Reads the manifests and makes sure each has a namespace set (when applicable) and adds annotations.
Expand All @@ -38,16 +40,14 @@ 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 rawManifests = (await readManifests(ctx, action, log, readFromSrcDir)) as KubernetesResource[]
const rawManifests = (await readManifests(ctx, action, log)) as KubernetesResource[]

// remove *List objects
const manifests = rawManifests.flatMap((manifest) => {
Expand All @@ -70,6 +70,11 @@ export async function getManifests({
return manifest
})

if (action.kind === "Deploy") {
// Add metadata ConfigMap to aid quick status check
manifests.push(getMetadataManifest(action, defaultNamespace, manifests))
}

return Bluebird.map(manifests, async (manifest) => {
// Ensure a namespace is set, if not already set, and if required by the resource type
if (!manifest.metadata?.namespace) {
Expand Down Expand Up @@ -105,33 +110,89 @@ export async function getManifests({
})
}

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,
manifests: KubernetesResource[]
): KubernetesResource<V1ConfigMap> {
const manifestMetadata: ManifestMetadata[] = manifests.map((m) => ({
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.message}`, error })
}
}

return output
}

const disallowedKustomizeArgs = ["-o", "--output", "-h", "--help"]

/**
* 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.
*/
export async function readManifests(
ctx: PluginContext,
action: Resolved<KubernetesDeployAction | KubernetesPodRunAction | KubernetesPodTestAction>,
log: Log,
readFromSrcDir = false
log: Log
) {
const manifestPath = readFromSrcDir ? action.basePath() : action.getBuildPath()
const manifestPath = action.getBuildPath()

const spec = action.getSpec()

const files = await glob(spec.files, { cwd: manifestPath })

const fileManifests = flatten(
await Bluebird.map(spec.files, async (path) => {
await Bluebird.map(files, async (path) => {
const absPath = resolve(manifestPath, path)
log.debug(`Reading manifest for module ${action.name} from path ${absPath}`)
log.debug(`Reading manifest for ${action.longDescription()} from path ${absPath}`)
const str = (await readFile(absPath)).toString()
const resolved = ctx.resolveTemplateStrings(str, { allowPartial: true, unescape: true })
return loadAll(resolved)
Expand Down
2 changes: 1 addition & 1 deletion core/src/plugins/kubernetes/kubernetes-type/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const kubernetesResourceSchema = () =>
.unknown(true)

export const kubernetesFilesSchema = () =>
joiSparseArray(joi.posixPath().subPathOnly()).description(
joiSparseArray(joi.posixPath().subPathOnly().allowGlobs()).description(
"POSIX-style paths to YAML files to load manifests from. Each can contain multiple manifests, and can include any Garden template strings, which will be resolved before applying the manifests."
)

Expand Down
Loading