Skip to content

Commit

Permalink
fix(provider): allow initialising providers without write ops for va…
Browse files Browse the repository at this point in the history
…lidation command (#6051)

* fix(provider): allow initialising providers without write ops for validation command

* fix(kubernetesProvider): fix environment readiness check

* chore: use own test project

* chore: add tests for terraform and exec providers

* chore: review suggestions
  • Loading branch information
twelvemo authored May 21, 2024
1 parent b8507a2 commit 2321ae8
Show file tree
Hide file tree
Showing 57 changed files with 354 additions and 166 deletions.
2 changes: 1 addition & 1 deletion core/src/commands/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class PluginsCommand extends Command<Args> {
printHeader(log, title, "⚙️")
}

const provider = await garden.resolveProvider(log, args.plugin)
const provider = await garden.resolveProvider({ log, name: args.plugin })
const ctx = await garden.getPluginContext({ provider, templateContext: undefined, events: undefined })

let graph = new ConfigGraph({
Expand Down
2 changes: 1 addition & 1 deletion core/src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class ValidateCommand extends Command<{}, Opts> {
async action({ garden, log, opts }: CommandParams<{}, Opts>): Promise<CommandResult> {
// This implicitly validates modules and actions.
const { resolve } = opts
const graph = await garden.getConfigGraph({ log, emit: false })
const graph = await garden.getConfigGraph({ log, emit: false, statusOnly: true })
if (resolve) {
const actionsToResolve = getActionsToResolve(resolve, graph)
await garden.resolveActions({ actions: actionsToResolve, graph, log })
Expand Down
43 changes: 30 additions & 13 deletions core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,19 @@ interface GardenInstanceState {
needsReload: boolean
}

interface ResolveProvidersParams {
log: Log
forceInit?: boolean
statusOnly?: boolean
names?: string[]
}

interface ResolveProviderParams {
log: Log
name: string
statusOnly?: boolean
}

@Profile()
export class Garden {
public log: Log
Expand Down Expand Up @@ -712,7 +725,8 @@ export class Garden {
: this.providerConfigs
}

async resolveProvider(log: Log, name: string) {
async resolveProvider(params: ResolveProviderParams): Promise<Provider> {
const { name, log, statusOnly } = params
if (name === "_default") {
return defaultProvider
}
Expand All @@ -723,7 +737,7 @@ export class Garden {

this.log.silly(() => `Resolving provider ${name}`)

const providers = await this.resolveProviders(log, false, [name])
const providers = await this.resolveProviders({ log, forceInit: false, statusOnly, names: [name] })
const provider = providers[name]

if (!provider) {
Expand All @@ -742,22 +756,23 @@ export class Garden {
@OtelTraced({
name: "resolveProviders",
})
async resolveProviders(log: Log, forceInit = false, names?: string[]): Promise<ProviderMap> {
async resolveProviders(params: ResolveProvidersParams): Promise<ProviderMap> {
const { log, forceInit = false, statusOnly = false, names } = params
// TODO: split this out of the Garden class
let providers: Provider[] = []
let providerNames = names

await this.asyncLock.acquire("resolve-providers", async () => {
const rawConfigs = this.getRawProviderConfigs({ names })

if (!names) {
names = getNames(rawConfigs)
if (!providerNames) {
providerNames = getNames(rawConfigs)
}

throwOnMissingSecretKeys(rawConfigs, this.secrets, "Provider", log)

// As an optimization, we return immediately if all requested providers are already resolved
const alreadyResolvedProviders = names.map((name) => this.resolvedProviders[name]).filter(Boolean)
if (alreadyResolvedProviders.length === names.length) {
const alreadyResolvedProviders = providerNames.map((name) => this.resolvedProviders[name]).filter(Boolean)
if (alreadyResolvedProviders.length === providerNames.length) {
providers = cloneDeep(alreadyResolvedProviders)
return
}
Expand Down Expand Up @@ -824,7 +839,7 @@ export class Garden {
})

// Process as many providers in parallel as possible
const taskResults = await this.processTasks({ tasks, log })
const taskResults = await this.processTasks({ tasks, log, statusOnly })

const providerResults = Object.values(taskResults.results.getMap())

Expand Down Expand Up @@ -920,7 +935,7 @@ export class Garden {
* Returns the reported status from all configured providers.
*/
async getEnvironmentStatus(log: Log) {
const providers = await this.resolveProviders(log)
const providers = await this.resolveProviders({ log })
return mapValues(providers, (p) => p.status)
}

Expand Down Expand Up @@ -961,7 +976,7 @@ export class Garden {
}

async getOutputConfigContext(log: Log, modules: GardenModule[], graphResults: GraphResults) {
const providers = await this.resolveProviders(log)
const providers = await this.resolveProviders({ log })
return new OutputConfigContext({
garden: this,
resolvedProviders: providers,
Expand Down Expand Up @@ -989,6 +1004,7 @@ export class Garden {
graphResults,
emit,
actionModes = {},
statusOnly,
/**
* If provided, this is used to perform partial module resolution.
* TODO: also limit action resolution (less important because it's faster and more done on-demand)
Expand All @@ -1003,7 +1019,7 @@ export class Garden {
// TODO: split this out of the Garden class
await this.scanAndAddConfigs()

const resolvedProviders = await this.resolveProviders(log)
const resolvedProviders = await this.resolveProviders({ log, forceInit: false, statusOnly })
const rawModuleConfigs = await this.getRawModuleConfigs()

const graphLog = log.createLog({ name: "graph", showDuration: true }).info(`Resolving actions and modules...`)
Expand Down Expand Up @@ -1693,7 +1709,7 @@ export class Garden {
await this.scanAndAddConfigs()

if (resolveProviders) {
providers = Object.values(await this.resolveProviders(log))
providers = Object.values(await this.resolveProviders({ log }))
} else {
providers = this.getRawProviderConfigs()
}
Expand Down Expand Up @@ -2280,5 +2296,6 @@ export interface GetConfigGraphParams {
graphResults?: GraphResults
emit: boolean
actionModes?: ActionModeMap
statusOnly?: boolean
actionsFilter?: string[]
}
2 changes: 1 addition & 1 deletion core/src/graph/nodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ export class ProcessTaskNode<T extends Task = Task> extends TaskNode<T> {
const processResult: TaskResultType<T> = await this.task.process({
status,
dependencyResults,
statusOnly: false,
statusOnly: this.statusOnly,
})
this.task.emit("processed", { result: processResult })
if (processResult.state === "ready") {
Expand Down
2 changes: 1 addition & 1 deletion core/src/graph/solver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ export class GraphSolver extends TypedEventEmitter<SolverEvents> {
// Status is either aborted or failed
this.log.silly(() => `Request ${request.getKey()} status: ${resultToString(status)}`)
this.completeTask({ ...status, node: request })
} else if (request.statusOnly && status !== undefined) {
} else if (request.statusOnly && status !== undefined && status.result) {
// Status is resolved, and that's all we need
this.log.silly(() => `Request ${request.getKey()} is statusOnly and the status is available. Completing.`)
this.completeTask({ ...status, node: request })
Expand Down
20 changes: 10 additions & 10 deletions core/src/plugins/kubernetes/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import { systemDockerAuthSecretName, dockerAuthSecretKey } from "./constants.js"
import type { V1IngressClass, V1Secret, V1Toleration } from "@kubernetes/client-node"
import type { KubernetesResource } from "./types.js"
import type { PrimitiveMap } from "../../config/common.js"
import { mapValues } from "lodash-es"
import { getIngressApiVersion, supportedIngressApiVersions } from "./container/ingress.js"
import type { Log } from "../../logger/log-entry.js"
import { ingressControllerInstall, ingressControllerReady } from "./nginx/ingress-controller.js"
Expand Down Expand Up @@ -74,30 +73,30 @@ export async function getEnvironmentStatus({
const provider = k8sCtx.provider
const api = await KubeApi.factory(log, ctx, provider)

const namespaces = await prepareNamespaces({ ctx, log })
const namespace = await getNamespaceStatus({ ctx: k8sCtx, log, provider: k8sCtx.provider, skipCreate: true })

const detail: KubernetesEnvironmentDetail = {
systemReady: true,
systemCertManagerReady: true,
systemManagedCertificatesReady: true,
}

const namespaceNames = mapValues(namespaces, (s) => s.namespaceName)
const result: KubernetesEnvironmentStatus = {
ready: true,
ready: namespace.state === "ready",
detail,
outputs: {
...namespaceNames,
"app-namespace": namespace.namespaceName,
"default-hostname": provider.config.defaultHostname || null,
},
}

if (provider.config.setupIngressController === "nginx") {
log.info(`Ensuring ${styles.highlight("nginx")} Ingress Controller...`)
const ingressControllerReadiness = await ingressControllerReady(ctx, log)
result.ready = ingressControllerReadiness
result.ready = ingressControllerReadiness && namespace.state === "ready"
detail.systemReady = ingressControllerReadiness
log.info(`${styles.highlight("nginx")} Ingress Controller ready`)
log.info(
`${styles.highlight("nginx")} Ingress Controller ${ingressControllerReadiness ? "is ready" : "is not ready"}`
)
} else {
// We only need to warn about missing ingress classes if we're not using garden installed nginx
const ingressApiVersion = await getIngressApiVersion(log, api, supportedIngressApiVersions)
Expand Down Expand Up @@ -145,7 +144,7 @@ export async function getIngressMisconfigurationWarnings(
}

/**
* Deploys system services (if any).
* Deploys system services (if any) and creates the default app namespace.
*/
export async function prepareEnvironment(
params: PrepareEnvironmentParams<KubernetesConfig, KubernetesEnvironmentStatus>
Expand All @@ -154,7 +153,8 @@ export async function prepareEnvironment(
const k8sCtx = <KubernetesPluginContext>ctx
const provider = k8sCtx.provider
const config = provider.config

// create default app namespace if it doesn't exist
await prepareNamespaces({ ctx, log })
// make sure that the system namespace exists
await getSystemNamespace(ctx, ctx.provider, log)

Expand Down
4 changes: 2 additions & 2 deletions core/src/router/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export abstract class BaseRouter {
templateContext: ConfigContext | undefined,
events: PluginEventBroker | undefined
): Promise<PluginActionParamsBase> {
const provider = await this.garden.resolveProvider(log, handler.pluginName)
const provider = await this.garden.resolveProvider({ log, name: handler.pluginName })

const ctx = await this.garden.getPluginContext({ provider, templateContext, events })

Expand Down Expand Up @@ -291,7 +291,7 @@ export abstract class BaseActionRouter<K extends ActionKind> extends BaseRouter
defaultHandler,
})

const providers = await this.garden.resolveProviders(log)
const providers = await this.garden.resolveProviders({ log })

const templateContext = action.isResolved()
? new ActionSpecContext({
Expand Down
2 changes: 1 addition & 1 deletion core/src/router/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export class ProviderRouter extends BaseRouter {
log.info("Cleaning up environments...")
const environmentStatuses: EnvironmentStatusMap = {}

const providers = await this.garden.resolveProviders(log)
const providers = await this.garden.resolveProviders({ log })
for (const provider of Object.values(providers)) {
await this.cleanupEnvironment({ pluginName: provider.name, log, events: undefined })
environmentStatuses[provider.name] = { ready: false, outputs: {} }
Expand Down
2 changes: 1 addition & 1 deletion core/src/tasks/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export class PublishTask extends BaseActionTask<BuildAction, PublishActionResult
let tagOverride: string | undefined = undefined

if (this.tagOverrideTemplate) {
const resolvedProviders = await this.garden.resolveProviders(this.log)
const resolvedProviders = await this.garden.resolveProviders({ log: this.log })

const templateContext = new BuildTagContext({
garden: this.garden,
Expand Down
6 changes: 3 additions & 3 deletions core/src/tasks/resolve-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export class ResolveActionTask<T extends Action> extends BaseActionTask<T, Resol
// Resolve template inputs
const inputsContext = new ActionSpecContext({
garden: this.garden,
resolvedProviders: await this.garden.resolveProviders(this.log),
resolvedProviders: await this.garden.resolveProviders({ log: this.log }),
action,
modules: this.graph.getModules(),
partialRuntimeResolution: false,
Expand Down Expand Up @@ -165,7 +165,7 @@ export class ResolveActionTask<T extends Action> extends BaseActionTask<T, Resol
}),
context: new ActionSpecContext({
garden: this.garden,
resolvedProviders: await this.garden.resolveProviders(this.log),
resolvedProviders: await this.garden.resolveProviders({ log: this.log }),
action,
modules: this.graph.getModules(),
partialRuntimeResolution: false,
Expand All @@ -188,7 +188,7 @@ export class ResolveActionTask<T extends Action> extends BaseActionTask<T, Resol
value: action.getConfig().spec || {},
context: new ActionSpecContext({
garden: this.garden,
resolvedProviders: await this.garden.resolveProviders(this.log),
resolvedProviders: await this.garden.resolveProviders({ log: this.log }),
action,
modules: this.graph.getModules(),
partialRuntimeResolution: false,
Expand Down
16 changes: 11 additions & 5 deletions core/src/tasks/resolve-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ export class ResolveProviderTask extends BaseTask<Provider> {
}
},
})
async process({ dependencyResults }: TaskProcessParams) {
async process({ dependencyResults, statusOnly }: TaskProcessParams) {
const providerResults = dependencyResults.getResultsByType(this).filter(isNotNull)
const resolvedProviders: ProviderMap = keyBy(providerResults.map((r) => r.result).filter(isNotNull), "name")

Expand Down Expand Up @@ -303,7 +303,8 @@ export class ResolveProviderTask extends BaseTask<Provider> {
moduleConfigs,
status: defaultEnvironmentStatus,
})
const status = await this.ensurePrepared(tmpProvider)

const status = await this.ensurePrepared(tmpProvider, statusOnly)

return providerFromConfig({
plugin: this.plugin,
Expand Down Expand Up @@ -379,7 +380,7 @@ export class ResolveProviderTask extends BaseTask<Provider> {
await writeFile(cachePath, serialize(cachedStatus))
}

private async ensurePrepared(tmpProvider: Provider) {
private async ensurePrepared(tmpProvider: Provider, statusOnly: boolean) {
const pluginName = tmpProvider.name
const providerLog = getProviderLog(pluginName, this.log)
const actions = await this.garden.getActionRouter()
Expand Down Expand Up @@ -416,7 +417,7 @@ export class ResolveProviderTask extends BaseTask<Provider> {

let status = await handler!({ ctx, log: providerLog })

if (this.forceInit || !status.ready) {
if (!statusOnly && (this.forceInit || !status.ready)) {
const statusMsg = status.ready
? `${styles.highlight("Ready")}, will ${styles.highlight("force re-initialize")}`
: `${styles.highlight("Not ready")}, will initialize`
Expand All @@ -433,12 +434,17 @@ export class ResolveProviderTask extends BaseTask<Provider> {
status = result.status
}

if (!status.ready) {
if (!status.ready && !statusOnly) {
providerLog.error("Failed initializing provider")
throw new PluginError({
message: `Provider ${pluginName} reports status as not ready and could not prepare the configured environment.`,
})
}

if (!status.ready && statusOnly) {
providerLog.success("Provider not ready. Current command only checks status, not preparing environment")
return status
}
providerLog.success("Provider ready")

if (!status.disableCache) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/util/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ export class TestGarden extends Garden {
name: string
status: ActionStatus<any>
}) {
const providers = await this.resolveProviders(log)
const providers = await this.resolveProviders({ log })

if (providers["test-plugin"]) {
set(providers["test-plugin"], ["_actionStatuses", kind, name], status)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: garden.io/v1
kind: Project
name: provider-handler-test
environments:
- name: local
providers:
- name: local-kubernetes
namespace: kubernetes-provider-handler-test
2 changes: 1 addition & 1 deletion core/test/integ/src/plugins/container/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe("plugins.container", () => {
beforeEach(async () => {
garden = await makeTestGarden(projectRoot, { plugins: [gardenPlugin()] })
log = createActionLog({ log: garden.log, actionName: "", actionKind: "" })
containerProvider = await garden.resolveProvider(garden.log, "container")
containerProvider = await garden.resolveProvider({ log: garden.log, name: "container" })
ctx = await garden.getPluginContext({ provider: containerProvider, templateContext: undefined, events: undefined })
td.replace(garden.buildStaging, "syncDependencyProducts", () => null)
})
Expand Down
2 changes: 1 addition & 1 deletion core/test/integ/src/plugins/exec/exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe("exec plugin", () => {

describe("provider outputs", () => {
it("the exec provider should have outputs for the initscript log", async () => {
const execProvider = await garden.resolveProvider(log, "exec")
const execProvider = await garden.resolveProvider({ log, name: "exec" })
expect((execProvider.outputs as ExecProviderOutputs).initScript.log).to.include(
"this is a provider output message"
)
Expand Down
Loading

0 comments on commit 2321ae8

Please sign in to comment.