-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/Add minimal k8s scheduler #105
Conversation
HadesScheduler/k8s/k8s.go
Outdated
type K8sConfig struct { | ||
// K8sNamespace is the namespace in which the jobs should be scheduled (default: hades-executor) | ||
// This may change in the future to allow for multiple namespaces | ||
K8sNamespace string `env:"K8S_NAMESPACE,notEmpty" envDefault:"hades-executor"` | ||
|
||
// K8sConfigMode is used to determine how the Kubernetes client should be configured ("kubeconfig" or "serviceaccount") | ||
ConfigMode string `env:"K8S_CONFIG_MODE,notEmpty" envDefault:"kubeconfig"` | ||
} | ||
|
||
// K8sConfigKubeconfig is used as configuration if used with a kubeconfig file | ||
type K8sConfigKubeconfig struct { | ||
kubeconfig string `env:"KUBECONFIG"` | ||
} | ||
|
||
// K8sConfigServiceaccount is used as configuration if used with a service account | ||
type K8sConfigServiceaccount struct { | ||
} |
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.
Personally, I prefer to have nested structs in this case. I mean in theory we can check for all env
variables and try to load them. Depending on the ConfigMode
we then select one of the nested structs
HadesScheduler/k8s/k8s.go
Outdated
job: job, | ||
k8sClient: k.k8sClient, | ||
namespace: k.namespace, | ||
sharedVolumeName: "shared", |
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.
Does it have side-effects on parallel builds if we name all the shared volumes the same?
7700dad
to
ec53594
Compare
Open Problems