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

Unify admission control plug-ins across providers #5142

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions cluster/aws/config-default.sh
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,6 @@ ENABLE_CLUSTER_DNS=true
DNS_SERVER_IP="10.0.0.10"
DNS_DOMAIN="kubernetes.local"
DNS_REPLICAS=1

# Admission Controllers to invoke prior to persisting objects in cluster
ADMISSION_CONTROL=NamespaceAutoProvision,LimitRanger,ResourceQuota
1 change: 1 addition & 0 deletions cluster/aws/templates/create-dynamic-salt-files.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ enable_cluster_dns: '$(echo "$ENABLE_CLUSTER_DNS" | sed -e "s/'/''/g")'
dns_replicas: '$(echo "$DNS_REPLICAS" | sed -e "s/'/''/g")'
dns_server: '$(echo "$DNS_SERVER_IP" | sed -e "s/'/''/g")'
dns_domain: '$(echo "$DNS_DOMAIN" | sed -e "s/'/''/g")'
admission_control: '$(echo "$ADMISSION_CONTROL" | sed -e "s/'/''/g")'
EOF

mkdir -p /srv/salt-overlay/salt/nginx
Expand Down
1 change: 1 addition & 0 deletions cluster/aws/util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ function kube-up {
echo "readonly DNS_REPLICAS='${DNS_REPLICAS:-}'"
echo "readonly DNS_SERVER_IP='${DNS_SERVER_IP:-}'"
echo "readonly DNS_DOMAIN='${DNS_DOMAIN:-}'"
echo "readonly ADMISSION_CONTROL='${ADMISSION_CONTROL:-}'"
grep -v "^#" "${KUBE_ROOT}/cluster/aws/templates/common.sh"
grep -v "^#" "${KUBE_ROOT}/cluster/aws/templates/create-dynamic-salt-files.sh"
grep -v "^#" "${KUBE_ROOT}/cluster/aws/templates/download-release.sh"
Expand Down
3 changes: 3 additions & 0 deletions cluster/azure/config-default.sh
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,6 @@ LOGGING_DESTINATION=elasticsearch # options: elasticsearch, gcp
# Optional: When set to true, Elasticsearch and Kibana will be setup as part of the cluster bring up.
ENABLE_CLUSTER_LOGGING=false
ELASTICSEARCH_LOGGING_REPLICAS=1

# Admission Controllers to invoke prior to persisting objects in cluster
ADMISSION_CONTROL=NamespaceAutoProvision,LimitRanger,ResourceQuota
1 change: 1 addition & 0 deletions cluster/azure/templates/create-dynamic-salt-files.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ cat <<EOF >/srv/salt-overlay/pillar/cluster-params.sls
instance_prefix: '$(echo "$INSTANCE_PREFIX" | sed -e "s/'/''/g")'
node_instance_prefix: $NODE_INSTANCE_PREFIX
portal_net: $PORTAL_NET
admission_control: '$(echo "$ADMISSION_CONTROL" | sed -e "s/'/''/g")'
EOF

mkdir -p /srv/salt-overlay/salt/nginx
Expand Down
1 change: 1 addition & 0 deletions cluster/azure/util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ function kube-up {
echo "readonly SALT_TAR_URL='${SALT_TAR_URL}'"
echo "readonly MASTER_HTPASSWD='${htpasswd}'"
echo "readonly PORTAL_NET='${PORTAL_NET}'"
echo "readonly ADMISSION_CONTROL='${ADMISSION_CONTROL:-}'"
grep -v "^#" "${KUBE_ROOT}/cluster/azure/templates/common.sh"
grep -v "^#" "${KUBE_ROOT}/cluster/azure/templates/create-dynamic-salt-files.sh"
grep -v "^#" "${KUBE_ROOT}/cluster/azure/templates/download-release.sh"
Expand Down
3 changes: 3 additions & 0 deletions cluster/gce/config-default.sh
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,6 @@ ENABLE_CLUSTER_DNS=true
DNS_SERVER_IP="10.0.0.10"
DNS_DOMAIN="kubernetes.local"
DNS_REPLICAS=1

# Admission Controllers to invoke prior to persisting objects in cluster
ADMISSION_CONTROL=NamespaceAutoProvision,LimitRanger,ResourceQuota
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what happens if these aren't defined? config-test.sh doesn't have them (which is what gce e2e uses), and I'm curious what values to use for GKE.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's easier to say what each does:

NamespaceAutoProvision ensures that if you create a resource in a namespace object that that namespace exists and if not, creates one. There is other lifecycle work around namespaces ongoing in other in flight PRs.

LimitRanger checks incoming pods and sees if there is a LimitRange in that namespace and if so, it validates pod resource usage (cpu, etc) are within defined min max constraints.

ResourceQuota enforces Hard usage limits in the namespace if that namespace has a quota applied.

Any miss to e2e was not intentional on my part. I think e2e should run with the same options.

Sent from my iPhone

On Mar 11, 2015, at 10:02 PM, Zach Loafman notifications@github.com wrote:

In cluster/gce/config-default.sh:

@@ -105,3 +105,6 @@ ENABLE_CLUSTER_DNS=true
DNS_SERVER_IP="10.0.0.10"
DNS_DOMAIN="kubernetes.local"
DNS_REPLICAS=1
+
+# Admission Controllers to invoke prior to persisting objects in cluster
+ADMISSION_CONTROL=NamespaceAutoProvision,LimitRanger,ResourceQuota
So what happens if these aren't defined? config-test.sh doesn't have them (which is what gce e2e uses), and I'm curious what values to use for GKE.


Reply to this email directly or view it on GitHub.

1 change: 1 addition & 0 deletions cluster/gce/configure-vm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ enable_cluster_dns: '$(echo "$ENABLE_CLUSTER_DNS" | sed -e "s/'/''/g")'
dns_replicas: '$(echo "$DNS_REPLICAS" | sed -e "s/'/''/g")'
dns_server: '$(echo "$DNS_SERVER_IP" | sed -e "s/'/''/g")'
dns_domain: '$(echo "$DNS_DOMAIN" | sed -e "s/'/''/g")'
admission_control: '$(echo "$ADMISSION_CONTROL" | sed -e "s/'/''/g")'
EOF

if [[ "${KUBERNETES_MASTER}" == "true" ]]; then
Expand Down
1 change: 1 addition & 0 deletions cluster/gce/util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ DNS_REPLICAS: $(yaml-quote ${DNS_REPLICAS:-})
DNS_SERVER_IP: $(yaml-quote ${DNS_SERVER_IP:-})
DNS_DOMAIN: $(yaml-quote ${DNS_DOMAIN:-})
MASTER_HTPASSWD: $(yaml-quote ${MASTER_HTPASSWD})
ADMISSION_CONTROL: $(yaml-quote ${ADMISSION_CONTROL:-})
EOF

if [[ "${master}" != "true" ]]; then
Expand Down
5 changes: 2 additions & 3 deletions cluster/saltbase/pillar/mine.sls
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% if grains.cloud is defined and grains.cloud == 'gce' -%}
# On GCE, there is no Salt mine. We run standalone.
{% if grains.cloud != 'gce' -%}

{% else %}
# Allow everyone to see cached values of who sits at what IP
{% set networkInterfaceName = "eth0" %}
{% if grains.networkInterfaceName is defined %}
Expand All @@ -9,5 +9,4 @@
mine_functions:
network.ip_addrs: [{{networkInterfaceName}}]
grains.items: []

{% endif -%}
4 changes: 2 additions & 2 deletions cluster/saltbase/salt/kube-apiserver/default
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@
{% endif -%}

{% set admission_control = "" -%}
{% if grains.admission_control is defined -%}
{% set admission_control = "--admission_control=" + grains.admission_control -%}
{% if pillar['admission_control'] is defined -%}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use of pillar unnecessary?

{% set admission_control = "--admission_control=" + pillar['admission_control'] -%}
{% endif -%}

{% set runtime_config = "" -%}
Expand Down
2 changes: 1 addition & 1 deletion cluster/vagrant/config-default.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ MASTER_USER=vagrant
MASTER_PASSWD=vagrant

# Admission Controllers to invoke prior to persisting objects in cluster
ADMISSION_CONTROL=NamespaceExists,LimitRanger,ResourceQuota,AlwaysAdmit
ADMISSION_CONTROL=NamespaceAutoProvision,LimitRanger,ResourceQuota

# Optional: Install node monitoring.
ENABLE_NODE_MONITORING=true
Expand Down
2 changes: 1 addition & 1 deletion cluster/vagrant/provision-master.sh
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ grains:
cloud_provider: vagrant
roles:
- kubernetes-master
admission_control: '$(echo "$ADMISSION_CONTROL" | sed -e "s/'/''/g")'
runtime_config: '$(echo "$RUNTIME_CONFIG" | sed -e "s/'/''/g")'
EOF

Expand All @@ -102,6 +101,7 @@ cat <<EOF >/srv/salt-overlay/pillar/cluster-params.sls
dns_server: '$(echo "$DNS_SERVER_IP" | sed -e "s/'/''/g")'
dns_domain: '$(echo "$DNS_DOMAIN" | sed -e "s/'/''/g")'
instance_prefix: '$(echo "$INSTANCE_PREFIX" | sed -e "s/'/''/g")'
admission_control: '$(echo "$ADMISSION_CONTROL" | sed -e "s/'/''/g")'
EOF

# Configure the salt-master
Expand Down
1 change: 1 addition & 0 deletions cluster/vagrant/provision-minion.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ EOF
# Our minions will have a pool role to distinguish them from the master.
cat <<EOF >/etc/salt/minion.d/grains.conf
grains:
cloud: vagrant
network_mode: openvswitch
node_ip: '$(echo "$MINION_IP" | sed -e "s/'/''/g")'
etcd_servers: '$(echo "$MASTER_IP" | sed -e "s/'/''/g")'
Expand Down
4 changes: 4 additions & 0 deletions plugin/pkg/admission/namespace/autoprovision/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ type provision struct {
}

func (p *provision) Admit(a admission.Attributes) (err error) {
// only handle create requests
if a.GetOperation() != "CREATE" {
return nil
}
defaultVersion, kind, err := latest.RESTMapper.VersionAndKindForResource(a.GetResource())
if err != nil {
return err
Expand Down
88 changes: 88 additions & 0 deletions plugin/pkg/admission/namespace/autoprovision/admission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,91 @@ limitations under the License.
*/

package autoprovision

import (
"testing"

"github.com/GoogleCloudPlatform/kubernetes/pkg/admission"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
"github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache"
)

// TestAdmission verifies a namespace is created on create requests for namespace managed resources
func TestAdmission(t *testing.T) {
namespace := "test"
mockClient := &client.Fake{}
handler := &provision{
client: mockClient,
store: cache.NewStore(cache.MetaNamespaceKeyFunc),
}
pod := api.Pod{
ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace},
Spec: api.PodSpec{
Volumes: []api.Volume{{Name: "vol"}},
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, namespace, "pods", "CREATE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
if len(mockClient.Actions) != 1 {
t.Errorf("Expected a create-namespace request")
}
if mockClient.Actions[0].Action != "create-namespace" {
t.Errorf("Expected a create-namespace request to be made via the client")
}
}

// TestAdmissionNamespaceExists verifies that no client call is made when a namespace already exists
func TestAdmissionNamespaceExists(t *testing.T) {
namespace := "test"
mockClient := &client.Fake{}
store := cache.NewStore(cache.MetaNamespaceKeyFunc)
store.Add(&api.Namespace{
ObjectMeta: api.ObjectMeta{Name: namespace},
})
handler := &provision{
client: mockClient,
store: store,
}
pod := api.Pod{
ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace},
Spec: api.PodSpec{
Volumes: []api.Volume{{Name: "vol"}},
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, namespace, "pods", "CREATE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
if len(mockClient.Actions) != 0 {
t.Errorf("No client request should have been made")
}
}

// TestIgnoreAdmission validates that a request is ignored if its not a create
func TestIgnoreAdmission(t *testing.T) {
namespace := "test"
mockClient := &client.Fake{}
handler := &provision{
client: mockClient,
store: cache.NewStore(cache.MetaNamespaceKeyFunc),
}
pod := api.Pod{
ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace},
Spec: api.PodSpec{
Volumes: []api.Volume{{Name: "vol"}},
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, namespace, "pods", "UPDATE"))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
if len(mockClient.Actions) != 0 {
t.Errorf("No client request should have been made")
}
}