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

SDK Service Account was Hardcoded #629

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
8 changes: 7 additions & 1 deletion cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const (
sidecarImageFlag = "sidecar-image"
sidecarCPURequestFlag = "sidecar-cpu-request"
sidecarCPULimitFlag = "sidecar-cpu-limit"
sdkServerAccountFlag = "sdk-service-account"
pullSidecarFlag = "always-pull-sidecar"
minPortFlag = "min-port"
maxPortFlag = "max-port"
Expand Down Expand Up @@ -152,7 +153,7 @@ func main() {

gsController := gameservers.NewController(wh, health,
ctlConf.MinPort, ctlConf.MaxPort, ctlConf.SidecarImage, ctlConf.AlwaysPullSidecar,
ctlConf.SidecarCPURequest, ctlConf.SidecarCPULimit,
ctlConf.SidecarCPURequest, ctlConf.SidecarCPULimit, ctlConf.SdkServiceAccount,
kubeClient, kubeInformerFactory, extClient, agonesClient, agonesInformerFactory)
gsSetController := gameserversets.NewController(wh, health,
kubeClient, extClient, agonesClient, agonesInformerFactory)
Expand Down Expand Up @@ -195,6 +196,7 @@ func parseEnvFlags() config {
viper.SetDefault(sidecarCPURequestFlag, "0")
viper.SetDefault(sidecarCPULimitFlag, "0")
viper.SetDefault(pullSidecarFlag, false)
viper.SetDefault(sdkServerAccountFlag, "agones-sdk")
viper.SetDefault(certFileFlag, filepath.Join(base, "certs/server.crt"))
viper.SetDefault(keyFileFlag, filepath.Join(base, "certs/server.key"))
viper.SetDefault(enablePrometheusMetricsFlag, true)
Expand All @@ -208,6 +210,7 @@ func parseEnvFlags() config {
pflag.String(sidecarCPULimitFlag, viper.GetString(sidecarCPULimitFlag), "Flag to overwrite the GameServer sidecar container's cpu limit. Can also use SIDECAR_CPU_LIMIT env variable")
pflag.String(sidecarCPURequestFlag, viper.GetString(sidecarCPURequestFlag), "Flag to overwrite the GameServer sidecar container's cpu request. Can also use SIDECAR_CPU_REQUEST env variable")
pflag.Bool(pullSidecarFlag, viper.GetBool(pullSidecarFlag), "For development purposes, set the sidecar image to have a ImagePullPolicy of Always. Can also use ALWAYS_PULL_SIDECAR env variable")
pflag.String(sdkServerAccountFlag, viper.GetString(sdkServerAccountFlag), "Overwrite what service account default for GameServer Pods. Defaults to Can also use SDK_SERVICE_ACCOUNT")
pflag.Int32(minPortFlag, 0, "Required. The minimum port that that a GameServer can be allocated to. Can also use MIN_PORT env variable.")
pflag.Int32(maxPortFlag, 0, "Required. The maximum port that that a GameServer can be allocated to. Can also use MAX_PORT env variable")
pflag.String(keyFileFlag, viper.GetString(keyFileFlag), "Optional. Path to the key file")
Expand All @@ -226,6 +229,7 @@ func parseEnvFlags() config {
runtime.Must(viper.BindEnv(sidecarCPULimitFlag))
runtime.Must(viper.BindEnv(sidecarCPURequestFlag))
runtime.Must(viper.BindEnv(pullSidecarFlag))
runtime.Must(viper.BindEnv(sdkServerAccountFlag))
runtime.Must(viper.BindEnv(minPortFlag))
runtime.Must(viper.BindEnv(maxPortFlag))
runtime.Must(viper.BindEnv(keyFileFlag))
Expand Down Expand Up @@ -255,6 +259,7 @@ func parseEnvFlags() config {
SidecarImage: viper.GetString(sidecarImageFlag),
SidecarCPURequest: request,
SidecarCPULimit: limit,
SdkServiceAccount: viper.GetString(sdkServerAccountFlag),
AlwaysPullSidecar: viper.GetBool(pullSidecarFlag),
KeyFile: viper.GetString(keyFileFlag),
CertFile: viper.GetString(certFileFlag),
Expand All @@ -275,6 +280,7 @@ type config struct {
SidecarImage string
SidecarCPURequest resource.Quantity
SidecarCPULimit resource.Quantity
SdkServiceAccount string
AlwaysPullSidecar bool
PrometheusMetrics bool
Stackdriver bool
Expand Down
2 changes: 2 additions & 0 deletions install/helm/agones/templates/controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ spec:
value: {{ .Values.agones.image.sdk.alwaysPull | quote }}
- name: SIDECAR_CPU_REQUEST
value: {{ .Values.agones.image.sdk.cpuRequest | quote }}
- name: SDK_SERVICE_ACCOUNT
value: {{ .Values.agones.serviceaccount.sdk | quote }}
- name: PROMETHEUS_EXPORTER
value: {{ .Values.agones.metrics.prometheusEnabled | quote }}
- name: STACKDRIVER_EXPORTER
Expand Down
2 changes: 2 additions & 0 deletions install/yaml/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,8 @@ spec:
value: "false"
- name: SIDECAR_CPU_REQUEST
value: "30m"
- name: SDK_SERVICE_ACCOUNT
value: "agones-sdk"
- name: PROMETHEUS_EXPORTER
value: "true"
- name: STACKDRIVER_EXPORTER
Expand Down
6 changes: 0 additions & 6 deletions pkg/apis/stable/v1alpha1/gameserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ const (
// GameServerContainerAnnotation is the annotation that stores
// which container is the container that runs the dedicated game server
GameServerContainerAnnotation = stable.GroupName + "/container"
// SidecarServiceAccountName is the default service account for managing access to get/update GameServers
SidecarServiceAccountName = "agones-sdk"
// DevAddressAnnotation is an annotation to indicate that a GameServer hosted outside of Agones.
// A locally hosted GameServer is not managed by Agones it is just simply registered.
DevAddressAnnotation = "stable.agones.dev/dev-address"
Expand Down Expand Up @@ -344,10 +342,6 @@ func (gs *GameServer) Pod(sidecars ...corev1.Container) (*corev1.Pod, error) {

gs.podObjectMeta(pod)

if pod.Spec.ServiceAccountName == "" {
pod.Spec.ServiceAccountName = SidecarServiceAccountName
}

i, gsContainer, err := gs.FindGameServerContainer()
// this shouldn't happen, but if it does.
if err != nil {
Expand Down
1 change: 0 additions & 1 deletion pkg/apis/stable/v1alpha1/gameserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,6 @@ func TestGameServerPod(t *testing.T) {
assert.Equal(t, "gameserver", pod.ObjectMeta.Labels[stable.GroupName+"/role"])
assert.Equal(t, fixture.ObjectMeta.Name, pod.ObjectMeta.Labels[GameServerPodLabel])
assert.Equal(t, fixture.Spec.Container, pod.ObjectMeta.Annotations[GameServerContainerAnnotation])
assert.Equal(t, "agones-sdk", pod.Spec.ServiceAccountName)
assert.True(t, metav1.IsControlledBy(pod, fixture))
assert.Equal(t, fixture.Spec.Ports[0].HostPort, pod.Spec.Containers[0].Ports[0].HostPort)
assert.Equal(t, fixture.Spec.Ports[0].ContainerPort, pod.Spec.Containers[0].Ports[0].ContainerPort)
Expand Down
8 changes: 8 additions & 0 deletions pkg/gameservers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type Controller struct {
alwaysPullSidecarImage bool
sidecarCPURequest resource.Quantity
sidecarCPULimit resource.Quantity
sdkServiceAccount string
crdGetter v1beta1.CustomResourceDefinitionInterface
podGetter typedcorev1.PodsGetter
podLister corelisterv1.PodLister
Expand Down Expand Up @@ -91,6 +92,7 @@ func NewController(
alwaysPullSidecarImage bool,
sidecarCPURequest resource.Quantity,
sidecarCPULimit resource.Quantity,
sdkServiceAccount string,
kubeClient kubernetes.Interface,
kubeInformerFactory informers.SharedInformerFactory,
extClient extclientset.Interface,
Expand All @@ -106,6 +108,7 @@ func NewController(
sidecarCPULimit: sidecarCPULimit,
sidecarCPURequest: sidecarCPURequest,
alwaysPullSidecarImage: alwaysPullSidecarImage,
sdkServiceAccount: sdkServiceAccount,
crdGetter: extClient.ApiextensionsV1beta1().CustomResourceDefinitions(),
podGetter: kubeClient.CoreV1(),
podLister: pods.Lister(),
Expand Down Expand Up @@ -512,6 +515,11 @@ func (c *Controller) createGameServerPod(gs *v1alpha1.GameServer) (*v1alpha1.Gam
return gs, err
}

// apply the sdk service account
if pod.Spec.ServiceAccountName == "" {
pod.Spec.ServiceAccountName = c.sdkServiceAccount
}

c.addGameServerHealthCheck(gs, pod)

c.logger.WithField("pod", pod).Info("creating Pod for GameServer")
Expand Down
4 changes: 3 additions & 1 deletion pkg/gameservers/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@ func TestControllerCreateGameServerPod(t *testing.T) {

assert.Equal(t, fixture.ObjectMeta.Name, pod.ObjectMeta.Name)
assert.Equal(t, fixture.ObjectMeta.Namespace, pod.ObjectMeta.Namespace)
assert.Equal(t, "sdk-service-account", pod.Spec.ServiceAccountName)
assert.Equal(t, "gameserver", pod.ObjectMeta.Labels[stable.GroupName+"/role"])
assert.Equal(t, fixture.ObjectMeta.Name, pod.ObjectMeta.Labels[v1alpha1.GameServerPodLabel])
assert.True(t, metav1.IsControlledBy(pod, fixture))
Expand Down Expand Up @@ -1185,7 +1186,8 @@ func newFakeController() (*Controller, agtesting.Mocks) {
wh := webhooks.NewWebHook("", "")
c := NewController(wh, healthcheck.NewHandler(),
10, 20, "sidecar:dev", false,
resource.MustParse("0.05"), resource.MustParse("0.1"), m.KubeClient, m.KubeInformerFactory, m.ExtClient, m.AgonesClient, m.AgonesInformerFactory)
resource.MustParse("0.05"), resource.MustParse("0.1"), "sdk-service-account",
m.KubeClient, m.KubeInformerFactory, m.ExtClient, m.AgonesClient, m.AgonesInformerFactory)
c.recorder = m.FakeRecorder
return c, m
}
Expand Down