-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add dhall-kubernetes
support for "schemas"
#84
Merged
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
let kubernetes = ../schemas.dhall | ||
|
||
let release = "wintering-rodent" | ||
|
||
let name = "aws-iam-authenticator" | ||
|
||
let fullName = "${release}-${name}" | ||
|
||
let version = "0.1.1" | ||
|
||
let chart = "${name}-${version}" | ||
|
||
let heritage = "dhall" | ||
|
||
in kubernetes.DaemonSet::{ | ||
, metadata = kubernetes.ObjectMeta::{ | ||
, name = "AWS IAM authenticator metadata" | ||
, labels = toMap | ||
{ app = name | ||
, chart = chart | ||
, release = release | ||
, heritage = heritage | ||
} | ||
} | ||
, spec = Some kubernetes.DaemonSetSpec::{ | ||
, updateStrategy = Some kubernetes.DaemonSetUpdateStrategy::{ | ||
, type = Some "RollingUpdate" | ||
} | ||
, template = kubernetes.PodTemplateSpec::{ | ||
, metadata = kubernetes.ObjectMeta::{ | ||
, name = name | ||
, annotations = toMap | ||
{ `scheduler.alpha.kubernetes.io/critical-pod` = "" | ||
} | ||
, labels = toMap | ||
{ app = name | ||
, release = release | ||
} | ||
} | ||
, spec = Some kubernetes.PodSpec::{ | ||
, hostNetwork = Some True | ||
, nodeSelector = toMap | ||
{ `node-role.kubernetes.io/master` = "" | ||
} | ||
, tolerations = | ||
[ kubernetes.Toleration::{ | ||
, effect = Some "NoSchedule" | ||
, key = Some "node-role.kubernetes.io/master" | ||
} | ||
, kubernetes.Toleration::{ | ||
, effect = Some "CriticalAddonsOnly" | ||
, key = Some "Exists" | ||
} | ||
] | ||
, containers = | ||
[ kubernetes.Container::{ | ||
, name = fullName | ||
, image = Some "gcr.io/heptio-images/authenticator:v0.1.0" | ||
, args = | ||
[ "server" | ||
, "--config=/etc/aws-iam-authenticator/config.yaml" | ||
, "--state-dir=/var/aws-iam-authenticator" | ||
, "--generate-kubeconfig=/etc/kubernetes/aws-iam-authenticator/kubeconfig.yaml" | ||
] | ||
, volumeMounts = | ||
[ kubernetes.VolumeMount::{ | ||
, name = "config" | ||
, mountPath = "/etc/aws-iam-authenticator/" | ||
} | ||
, kubernetes.VolumeMount::{ | ||
, name = "state" | ||
, mountPath = "/var/aws-iam-authenticator/" | ||
} | ||
, kubernetes.VolumeMount::{ | ||
, name = "output" | ||
, mountPath = "/etc/kubernetes/aws-iam-authenticator/" | ||
} | ||
] | ||
} | ||
] | ||
, volumes = | ||
[ kubernetes.Volume::{ | ||
, name = "config" | ||
, configMap = Some kubernetes.ConfigMapVolumeSource::{ | ||
, name = Some fullName | ||
} | ||
} | ||
, kubernetes.Volume::{ | ||
, name = "output" | ||
, hostPath = Some kubernetes.HostPathVolumeSource::{ | ||
, path = "/srv/kubernetes/aws-iam-authenticator/" | ||
} | ||
} | ||
, kubernetes.Volume::{ | ||
, name = "state" | ||
, hostPath = Some kubernetes.HostPathVolumeSource::{ | ||
, path = "/srv/kubernetes/aws-iam-authenticator/" | ||
} | ||
} | ||
] | ||
} | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one of the places where I think you can ommit the
metadata.name
attributeThis is a manifestation of #8 (comment) I think the ground rule is that only toplevel objects need
name
, any template objects can (and must?) omit it instead.I think if you explicitly set
metadata.name
here, really weird things might happen. Every pod will have the same name, e.g. there will never be more than 1 pod created eventhough the daemonset wants to create more than 1.Also I think you must set
template.selector
otherwise thepods
that thedaemonset
will create will not associate with thedaemonset
(See https://kubernetes.io/docs/concepts/workloads/controllers/daemonset/)It used to be the case that if you left out
selector
that it would be implicitly created, but I think that behaviour was removed recently. At least forDeployment
; maybe not fordaemonset
.I'm poking @akshaymankar here just to be sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bug that we should fix.
name
should not be a required attribute ofObjectMeta
. The bug issue is #8There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I only added the
name
field because the type required itFor reference, this is based off of: https://github.com/helm/charts/blob/550096fcda27d7637a7a066240c61a4c6cb61f21/stable/aws-iam-authenticator/templates/daemonset.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeh that chart uses an older version of the
DaemonSet
API. Ours requires theselector
: https://github.com/dhall-lang/dhall-kubernetes/blob/master/types/io.k8s.api.apps.v1.DaemonSetSpec.dhall so your example will hopefully give you a nice type error! (Woohoo dhall types!)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arianvp: The example type-checks because the default record for
DaemonSetSpec
defines an emptyselector
fieldThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh yes. I remember now. We have a bug open for this as well :) #78
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked with a daemonset, nothing weird happened, the controller just ignored the
name
field while creating pods. But it might confuse somebody reading a config.This is true, if you don't set it, it is a validation error, but only for daemonsets. Not sure if this should be a type error given the OpenAPI spec doesn't say anything about it.