Skip to content

Commit

Permalink
Merge pull request #491 from garden-io/container-status-fix
Browse files Browse the repository at this point in the history 
fix(status): return more correct/granular statuses
  • Loading branch information
@eysi09
eysi09 authored Jan 31, 2019
2 parents 3f7d9c2 + d4a7cf2 commit abd6509
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 77 deletions.
4 changes: 3 additions & 1 deletion garden-service/src/plugins/kubernetes/container/status.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { ContainerModule } from "../../container/config"
import { KubeApi } from "../api"
import { compareDeployedObjects } from "../status"
import { getIngresses } from "./ingress"
import { getAppNamespace } from "../namespace"

export async function getContainerServiceStatus(
{ ctx, module, service, runtimeContext, log, hotReload }: GetServiceStatusParams<ContainerModule>,
Expand All @@ -26,10 +27,11 @@ export async function getContainerServiceStatus(
// TODO: hash and compare all the configuration files (otherwise internal changes don't get deployed)
const version = module.version
const api = new KubeApi(ctx.provider)
const namespace = await getAppNamespace(ctx, ctx.provider)

// FIXME: [objects, matched] and ingresses can be run in parallel
const objects = await createContainerObjects(ctx, service, runtimeContext, hotReload)
const { state, remoteObjects } = await compareDeployedObjects(ctx, objects, log)
const { state, remoteObjects } = await compareDeployedObjects(ctx, api, namespace, objects, log)
const ingresses = await getIngresses(service, api)

return {
Expand Down
20 changes: 5 additions & 15 deletions garden-service/src/plugins/kubernetes/helm/status.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { ServiceStatus, ServiceState, combineStates } from "../../../types/service"
import { ServiceStatus, ServiceState } from "../../../types/service"
import { GetServiceStatusParams, GetServiceOutputsParams } from "../../../types/plugin/params"
import { getExecModuleBuildStatus } from "../../exec"
import { compareDeployedObjects, checkResourceStatuses } from "../status"
import { compareDeployedObjects } from "../status"
import { KubeApi } from "../api"
import { getAppNamespace } from "../namespace"
import { LogEntry } from "../../../logger/log-entry"
Expand Down Expand Up @@ -59,24 +59,14 @@ export async function getServiceStatus(
})
}

let { state, remoteObjects } = await compareDeployedObjects(ctx, chartResources, log)
const detail = { remoteObjects }

if (state !== "ready") {
return { state }
}

// then check if the rollout is complete
const version = module.version
const api = new KubeApi(ctx.provider)
const namespace = await getAppNamespace(ctx, ctx.provider)
const statuses = await checkResourceStatuses(api, namespace, chartResources)

state = combineStates(statuses.map(s => s.state))
let { state, remoteObjects } = await compareDeployedObjects(ctx, api, namespace, chartResources, log)
const detail = { remoteObjects }

return {
state,
version: version.versionString,
version: state === "ready" ? module.version.versionString : undefined,
detail,
}
}
Expand Down
152 changes: 91 additions & 61 deletions garden-service/src/plugins/kubernetes/status.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,27 +295,32 @@ export async function checkResourceStatuses(
api: KubeApi, namespace: string, resources: KubernetesResource[], prevStatuses?: RolloutStatus[],
): Promise<RolloutStatus[]> {
return Bluebird.map(resources, async (obj, i) => {
const handler = objHandlers[obj.kind]
const prevStatus = prevStatuses && prevStatuses[i]
let status: RolloutStatus
if (handler) {
try {
status = await handler(api, namespace, obj, prevStatus && prevStatus.resourceVersion)
} catch (err) {
// We handle 404s specifically since this might be invoked before some objects are deployed
if (err.code === 404) {
status = { state: "missing", obj }
} else {
throw err
}
return checkResourceStatus(api, namespace, obj, prevStatuses && prevStatuses[i])
})
}

export async function checkResourceStatus(
api: KubeApi, namespace: string, resource: KubernetesResource, prevStatus?: RolloutStatus,
) {
const handler = objHandlers[resource.kind]
let status: RolloutStatus
if (handler) {
try {
status = await handler(api, namespace, resource, prevStatus && prevStatus.resourceVersion)
} catch (err) {
// We handle 404s specifically since this might be invoked before some objects are deployed
if (err.code === 404) {
status = { state: "missing", obj: resource }
} else {
throw err
}
} else {
// if there is no explicit handler to check the status, we assume there's no rollout phase to wait for
status = { state: "ready", obj }
}
} else {
// if there is no explicit handler to check the status, we assume there's no rollout phase to wait for
status = { state: "ready", obj: resource }
}

return status
})
return status
}

interface WaitParams {
Expand Down Expand Up @@ -402,60 +407,88 @@ interface ComparisonResult {
* Check if each of the given Kubernetes objects matches what's installed in the cluster
*/
export async function compareDeployedObjects(
ctx: PluginContext, objects: KubernetesResource[], log: LogEntry,
ctx: PluginContext, api: KubeApi, namespace: string, objects: KubernetesResource[], log: LogEntry,
): Promise<ComparisonResult> {
const existingObjects = await Bluebird.map(objects, obj => getDeployedObject(ctx, ctx.provider, obj))

let missing = true
const maybeDeployedObjects = await Bluebird.map(objects, obj => getDeployedObject(ctx, ctx.provider, obj))
const deployedObjects = <KubernetesResource[]>maybeDeployedObjects.filter(o => o !== null)

const result: ComparisonResult = {
state: "ready",
remoteObjects: <KubernetesResource[]>existingObjects.filter(o => o !== null),
state: "unknown",
remoteObjects: <KubernetesResource[]>deployedObjects.filter(o => o !== null),
}

for (let [newSpec, existingSpec] of zip(objects, existingObjects)) {
if (newSpec && !existingSpec) {
log.silly(`Resource ${newSpec.kind}/${newSpec.metadata.name} missing from cluster`)
}
const logDescription = (obj: KubernetesResource) => `${obj.kind}/${obj.metadata.name}`

if (existingSpec && newSpec) {
missing = false
const missingObjectNames = zip(objects, maybeDeployedObjects)
.filter(([_, deployed]) => !deployed)
.map(([obj, _]) => logDescription(obj!))

// the API version may implicitly change when deploying
existingSpec.apiVersion = newSpec.apiVersion
if (missingObjectNames.length > 0) {
// One or more objects is not deployed.
log.silly(`Resource(s) ${missingObjectNames.join(", ")} missing from cluster`)
result.state = "missing"
return result
}

// the namespace property is silently dropped when added to non-namespaced
if (newSpec.metadata.namespace && existingSpec.metadata.namespace === undefined) {
delete newSpec.metadata.namespace
}
const deployedObjectStatuses: RolloutStatus[] = await Bluebird.map(
deployedObjects,
async (obj) => checkResourceStatus(api, namespace, obj, undefined))

if (!existingSpec.metadata.annotations) {
existingSpec.metadata.annotations = {}
}
const deployedStates = deployedObjectStatuses.map(s => s.state)
if (deployedStates.find(s => s !== "ready")) {

// handle auto-filled properties (this is a bit of a design issue in the K8s API)
if (newSpec.kind === "Service" && newSpec.spec.clusterIP === "") {
delete newSpec.spec.clusterIP
}
const descriptions = zip(deployedObjects, deployedStates)
.filter(([_, s]) => s !== "ready")
.map(([o, s]) => `${logDescription(o!)}: "${s}"`).join("\n")

// handle properties that are omitted in the response because they have the default value
// (another design issue in the K8s API)
// NOTE: this approach won't fly in the long run, but hopefully we can climb out of this mess when
// `kubectl diff` is ready, or server-side apply/diff is ready
if (newSpec.kind === "DaemonSet") {
if (newSpec.spec.minReadySeconds === 0) {
delete newSpec.spec.minReadySeconds
}
if (newSpec.spec.template.spec.hostNetwork === false) {
delete newSpec.spec.template.spec.hostNetwork
}
}
log.silly(dedent`
Resource(s) with non-ready status found in the cluster:
// clean null values
newSpec = <KubernetesResource>removeNull(newSpec)
${descriptions}` + "\n")

result.state = combineStates(deployedStates)
return result
}

// From here, the state can only be "ready" or "outdated", so we proceed to compare the old & new specs.

for (let [newSpec, existingSpec] of zip(objects, deployedObjects) as KubernetesResource[][]) {

// the API version may implicitly change when deploying
existingSpec.apiVersion = newSpec.apiVersion

// the namespace property is silently dropped when added to non-namespaced
if (newSpec.metadata.namespace && existingSpec.metadata.namespace === undefined) {
delete newSpec.metadata.namespace
}

if (!existingSpec.metadata.annotations) {
existingSpec.metadata.annotations = {}
}

// handle auto-filled properties (this is a bit of a design issue in the K8s API)
if (newSpec.kind === "Service" && newSpec.spec.clusterIP === "") {
delete newSpec.spec.clusterIP
}

// handle properties that are omitted in the response because they have the default value
// (another design issue in the K8s API)
// NOTE: this approach won't fly in the long run, but hopefully we can climb out of this mess when
// `kubectl diff` is ready, or server-side apply/diff is ready
if (newSpec.kind === "DaemonSet") {
if (newSpec.spec.minReadySeconds === 0) {
delete newSpec.spec.minReadySeconds
}
if (newSpec.spec.template.spec.hostNetwork === false) {
delete newSpec.spec.template.spec.hostNetwork
}
}

if (existingSpec && !isSubset(existingSpec, newSpec)) {
// clean null values
newSpec = <KubernetesResource>removeNull(newSpec)

if (!isSubset(existingSpec, newSpec)) {
if (newSpec) {
log.silly(`Resource ${newSpec.metadata.name} is not a superset of deployed resource`)
log.silly("----------------- Expected: -----------------------")
Expand All @@ -473,10 +506,7 @@ export async function compareDeployedObjects(
}
}

if (missing) {
result.state = "missing"
}

result.state = "ready"
return result
}

Expand Down

0 comments on commit abd6509

Please sign in to comment.