Browse code

Project node selector fixes

- Allow empty project node selector
- Changed "openshift.io/node-selector" annotation as constant

Ravi Sankar Penta authored on 2015/05/23 07:53:54
Showing 14 changed files
... ...
@@ -54,7 +54,12 @@ func NewCmdNewProject(name, fullName string, f *clientcmd.Factory, out io.Writer
54 54
 			if options.Client, _, err = f.Clients(); err != nil {
55 55
 				kcmdutil.CheckErr(err)
56 56
 			}
57
-			if err := options.Run(); err != nil {
57
+
58
+			// We can't depend on len(options.NodeSelector) > 0 as node-selector="" is valid
59
+			// and we want to populate node selector as project annotation only if explicitly set by user
60
+			useNodeSelector := cmd.Flag("node-selector").Changed
61
+
62
+			if err := options.Run(useNodeSelector); err != nil {
58 63
 				kcmdutil.CheckErr(err)
59 64
 			}
60 65
 		},
... ...
@@ -64,7 +69,7 @@ func NewCmdNewProject(name, fullName string, f *clientcmd.Factory, out io.Writer
64 64
 	cmd.Flags().StringVar(&options.AdminUser, "admin", "", "project admin username")
65 65
 	cmd.Flags().StringVar(&options.DisplayName, "display-name", "", "project display name")
66 66
 	cmd.Flags().StringVar(&options.Description, "description", "", "project description")
67
-	cmd.Flags().StringVar(&options.NodeSelector, "node-selector", "", "Restrict pods onto nodes matching given label selector. Format: '<key1>=<value1>, <key2>=<value2>...'")
67
+	cmd.Flags().StringVar(&options.NodeSelector, "node-selector", "", "Restrict pods onto nodes matching given label selector. Format: '<key1>=<value1>, <key2>=<value2>...'. Specifying \"\" means any node, not default. If unspecified, cluster default node selector will be used.")
68 68
 
69 69
 	return cmd
70 70
 }
... ...
@@ -78,7 +83,7 @@ func (o *NewProjectOptions) complete(args []string) error {
78 78
 	return nil
79 79
 }
80 80
 
81
-func (o *NewProjectOptions) Run() error {
81
+func (o *NewProjectOptions) Run(useNodeSelector bool) error {
82 82
 	if _, err := o.Client.Projects().Get(o.ProjectName); err != nil {
83 83
 		if !kerrors.IsNotFound(err) {
84 84
 			return err
... ...
@@ -92,7 +97,9 @@ func (o *NewProjectOptions) Run() error {
92 92
 	project.Annotations = make(map[string]string)
93 93
 	project.Annotations["description"] = o.Description
94 94
 	project.Annotations["displayName"] = o.DisplayName
95
-	project.Annotations["openshift.io/node-selector"] = o.NodeSelector
95
+	if useNodeSelector {
96
+		project.Annotations[projectapi.ProjectNodeSelectorParam] = o.NodeSelector
97
+	}
96 98
 	project, err := o.Client.Projects().Create(project)
97 99
 	if err != nil {
98 100
 		return err
... ...
@@ -18,10 +18,9 @@ import (
18 18
 )
19 19
 
20 20
 type NewProjectOptions struct {
21
-	ProjectName  string
22
-	DisplayName  string
23
-	Description  string
24
-	NodeSelector string
21
+	ProjectName string
22
+	DisplayName string
23
+	Description string
25 24
 
26 25
 	Client client.Interface
27 26
 
... ...
@@ -106,7 +105,6 @@ func (o *NewProjectOptions) Run() error {
106 106
 	projectRequest.DisplayName = o.DisplayName
107 107
 	projectRequest.Annotations = make(map[string]string)
108 108
 	projectRequest.Annotations["description"] = o.Description
109
-	projectRequest.Annotations["openshift.io/node-selector"] = o.NodeSelector
110 109
 
111 110
 	project, err := o.Client.ProjectRequests().Create(projectRequest)
112 111
 	if err != nil {
... ...
@@ -27,6 +27,7 @@ import (
27 27
 	buildutil "github.com/openshift/origin/pkg/build/util"
28 28
 	"github.com/openshift/origin/pkg/client"
29 29
 	imageapi "github.com/openshift/origin/pkg/image/api"
30
+	projectapi "github.com/openshift/origin/pkg/project/api"
30 31
 	templateapi "github.com/openshift/origin/pkg/template/api"
31 32
 )
32 33
 
... ...
@@ -552,7 +553,7 @@ func (d *ProjectDescriber) Describe(namespace, name string) (string, error) {
552 552
 
553 553
 	nodeSelector := ""
554 554
 	if len(project.ObjectMeta.Annotations) > 0 {
555
-		if ns, ok := project.ObjectMeta.Annotations["openshift.io/node-selector"]; ok {
555
+		if ns, ok := project.ObjectMeta.Annotations[projectapi.ProjectNodeSelectorParam]; ok {
556 556
 			nodeSelector = ns
557 557
 		}
558 558
 	}
... ...
@@ -8,6 +8,7 @@ import (
8 8
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache"
9 9
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/testclient"
10 10
 
11
+	projectapi "github.com/openshift/origin/pkg/project/api"
11 12
 	projectcache "github.com/openshift/origin/pkg/project/cache"
12 13
 	"github.com/openshift/origin/pkg/util/labelselector"
13 14
 )
... ...
@@ -30,28 +31,29 @@ func TestPodAdmission(t *testing.T) {
30 30
 	}
31 31
 
32 32
 	tests := []struct {
33
-		defaultNodeSelector string
34
-		projectNodeSelector string
35
-		podNodeSelector     map[string]string
36
-		mergedNodeSelector  map[string]string
37
-		admit               bool
38
-		testName            string
33
+		defaultNodeSelector       string
34
+		projectNodeSelector       string
35
+		podNodeSelector           map[string]string
36
+		mergedNodeSelector        map[string]string
37
+		ignoreProjectNodeSelector bool
38
+		admit                     bool
39
+		testName                  string
39 40
 	}{
40 41
 		{
41
-			defaultNodeSelector: "",
42
-			projectNodeSelector: "",
43
-			podNodeSelector:     map[string]string{},
44
-			mergedNodeSelector:  map[string]string{},
45
-			admit:               true,
46
-			testName:            "No node selectors",
42
+			defaultNodeSelector:       "",
43
+			podNodeSelector:           map[string]string{},
44
+			mergedNodeSelector:        map[string]string{},
45
+			ignoreProjectNodeSelector: true,
46
+			admit:    true,
47
+			testName: "No node selectors",
47 48
 		},
48 49
 		{
49
-			defaultNodeSelector: "infra = false",
50
-			projectNodeSelector: "",
51
-			podNodeSelector:     map[string]string{},
52
-			mergedNodeSelector:  map[string]string{"infra": "false"},
53
-			admit:               true,
54
-			testName:            "Default node selector and no conflicts",
50
+			defaultNodeSelector:       "infra = false",
51
+			podNodeSelector:           map[string]string{},
52
+			mergedNodeSelector:        map[string]string{"infra": "false"},
53
+			ignoreProjectNodeSelector: true,
54
+			admit:    true,
55
+			testName: "Default node selector and no conflicts",
55 56
 		},
56 57
 		{
57 58
 			defaultNodeSelector: "",
... ...
@@ -63,6 +65,14 @@ func TestPodAdmission(t *testing.T) {
63 63
 		},
64 64
 		{
65 65
 			defaultNodeSelector: "infra = false",
66
+			projectNodeSelector: "",
67
+			podNodeSelector:     map[string]string{},
68
+			mergedNodeSelector:  map[string]string{},
69
+			admit:               true,
70
+			testName:            "Empty project node selector and no conflicts",
71
+		},
72
+		{
73
+			defaultNodeSelector: "infra = false",
66 74
 			projectNodeSelector: "infra=true",
67 75
 			podNodeSelector:     map[string]string{},
68 76
 			mergedNodeSelector:  map[string]string{"infra": "true"},
... ...
@@ -96,7 +106,9 @@ func TestPodAdmission(t *testing.T) {
96 96
 	}
97 97
 	for _, test := range tests {
98 98
 		projectcache.FakeProjectCache(mockClient, projectStore, test.defaultNodeSelector)
99
-		project.ObjectMeta.Annotations = map[string]string{"openshift.io/node-selector": test.projectNodeSelector}
99
+		if !test.ignoreProjectNodeSelector {
100
+			project.ObjectMeta.Annotations = map[string]string{projectapi.ProjectNodeSelectorParam: test.projectNodeSelector}
101
+		}
100 102
 		pod.Spec = kapi.PodSpec{NodeSelector: test.podNodeSelector}
101 103
 
102 104
 		err := handler.Admit(admission.NewAttributesRecord(pod, "Pod", project.ObjectMeta.Name, "pods", admission.Create, nil))
... ...
@@ -11,9 +11,11 @@ type ProjectList struct {
11 11
 	Items []Project
12 12
 }
13 13
 
14
-// These are internal finalizer values to Origin
15 14
 const (
15
+	// These are internal finalizer values to Origin
16 16
 	FinalizerOrigin kapi.FinalizerName = "openshift.io/origin"
17
+
18
+	ProjectNodeSelectorParam string = "openshift.io/node-selector"
17 19
 )
18 20
 
19 21
 // ProjectSpec describes the attributes on a Project
... ...
@@ -11,9 +11,11 @@ type ProjectList struct {
11 11
 	Items         []Project `json:"items"`
12 12
 }
13 13
 
14
-// These are internal finalizer values to Origin
15 14
 const (
15
+	// These are internal finalizer values to Origin
16 16
 	FinalizerOrigin kapi.FinalizerName = "openshift.io/origin"
17
+
18
+	ProjectNodeSelectorParam string = "openshift.io/node-selector"
17 19
 )
18 20
 
19 21
 // ProjectSpec describes the attributes on a Project
... ...
@@ -11,9 +11,11 @@ type ProjectList struct {
11 11
 	Items         []Project `json:"items"`
12 12
 }
13 13
 
14
-// These are internal finalizer values to Origin
15 14
 const (
15
+	// These are internal finalizer values to Origin
16 16
 	FinalizerOrigin kapi.FinalizerName = "openshift.io/origin"
17
+
18
+	ProjectNodeSelectorParam string = "openshift.io/node-selector"
17 19
 )
18 20
 
19 21
 // ProjectSpec describes the attributes on a Project
... ...
@@ -11,9 +11,11 @@ type ProjectList struct {
11 11
 	Items         []Project `json:"items"`
12 12
 }
13 13
 
14
-// These are internal finalizer values to Origin
15 14
 const (
15
+	// These are internal finalizer values to Origin
16 16
 	FinalizerOrigin kapi.FinalizerName = "openshift.io/origin"
17
+
18
+	ProjectNodeSelectorParam string = "openshift.io/node-selector"
17 19
 )
18 20
 
19 21
 // ProjectSpec describes the attributes on a Project
... ...
@@ -59,9 +59,9 @@ func validateNodeSelector(p *api.Project) fielderrors.ValidationErrorList {
59 59
 	allErrs := fielderrors.ValidationErrorList{}
60 60
 
61 61
 	if len(p.Annotations) > 0 {
62
-		if selector, ok := p.Annotations["openshift.io/node-selector"]; ok {
62
+		if selector, ok := p.Annotations[api.ProjectNodeSelectorParam]; ok {
63 63
 			if _, err := labelselector.Parse(selector); err != nil {
64
-				allErrs = append(allErrs, fielderrors.NewFieldInvalid("nodeSelector", p.Annotations["openshift.io/node-selector"], "must be a valid label selector"))
64
+				allErrs = append(allErrs, fielderrors.NewFieldInvalid("nodeSelector", p.Annotations[api.ProjectNodeSelectorParam], "must be a valid label selector"))
65 65
 			}
66 66
 		}
67 67
 	}
... ...
@@ -122,7 +122,7 @@ func TestValidateProject(t *testing.T) {
122 122
 					Name:      "foo",
123 123
 					Namespace: "",
124 124
 					Annotations: map[string]string{
125
-						"openshift.io/node-selector": "infra=true, env = test",
125
+						api.ProjectNodeSelectorParam: "infra=true, env = test",
126 126
 					},
127 127
 				},
128 128
 			},
... ...
@@ -135,7 +135,7 @@ func TestValidateProject(t *testing.T) {
135 135
 					Name:      "foo",
136 136
 					Namespace: "",
137 137
 					Annotations: map[string]string{
138
-						"openshift.io/node-selector": "infra, env = $test",
138
+						api.ProjectNodeSelectorParam: "infra, env = $test",
139 139
 					},
140 140
 				},
141 141
 			},
... ...
@@ -11,6 +11,7 @@ import (
11 11
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
12 12
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
13 13
 
14
+	projectapi "github.com/openshift/origin/pkg/project/api"
14 15
 	"github.com/openshift/origin/pkg/util/labelselector"
15 16
 )
16 17
 
... ...
@@ -51,12 +52,14 @@ func (p *ProjectCache) GetNamespaceObject(name string) (*kapi.Namespace, error)
51 51
 
52 52
 func (p *ProjectCache) GetNodeSelector(namespace *kapi.Namespace) string {
53 53
 	selector := ""
54
+	found := false
54 55
 	if len(namespace.ObjectMeta.Annotations) > 0 {
55
-		if ns, ok := namespace.ObjectMeta.Annotations["openshift.io/node-selector"]; ok {
56
+		if ns, ok := namespace.ObjectMeta.Annotations[projectapi.ProjectNodeSelectorParam]; ok {
56 57
 			selector = ns
58
+			found = true
57 59
 		}
58 60
 	}
59
-	if len(selector) == 0 {
61
+	if !found {
60 62
 		selector = p.DefaultNodeSelector
61 63
 	}
62 64
 	return selector
... ...
@@ -66,7 +66,7 @@ func TestLogin(t *testing.T) {
66 66
 		AdminRole:   bootstrappolicy.AdminRoleName,
67 67
 		AdminUser:   username,
68 68
 	}
69
-	if err := newProjectOptions.Run(); err != nil {
69
+	if err := newProjectOptions.Run(false); err != nil {
70 70
 		t.Fatalf("unexpected error, a project is required to continue: %v", err)
71 71
 	}
72 72
 
... ...
@@ -131,8 +131,8 @@ func TestProjectIsNamespace(t *testing.T) {
131 131
 		ObjectMeta: kapi.ObjectMeta{
132 132
 			Name: "new-project",
133 133
 			Annotations: map[string]string{
134
-				"displayName":                "Hello World",
135
-				"openshift.io/node-selector": "env=test",
134
+				"displayName":                       "Hello World",
135
+				projectapi.ProjectNodeSelectorParam: "env=test",
136 136
 			},
137 137
 		},
138 138
 	}
... ...
@@ -152,8 +152,8 @@ func TestProjectIsNamespace(t *testing.T) {
152 152
 	if project.Annotations["displayName"] != namespace.Annotations["displayName"] {
153 153
 		t.Fatalf("Project display name did not match namespace annotation, project %v, namespace %v", project.Annotations["displayName"], namespace.Annotations["displayName"])
154 154
 	}
155
-	if project.Annotations["openshift.io/node-selector"] != namespace.Annotations["openshift.io/node-selector"] {
156
-		t.Fatalf("Project node selector did not match namespace node selector, project %v, namespace %v", project.Annotations["openshift.io/node-selector"], namespace.Annotations["openshift.io/node-selector"])
155
+	if project.Annotations[projectapi.ProjectNodeSelectorParam] != namespace.Annotations[projectapi.ProjectNodeSelectorParam] {
156
+		t.Fatalf("Project node selector did not match namespace node selector, project %v, namespace %v", project.Annotations[projectapi.ProjectNodeSelectorParam], namespace.Annotations[projectapi.ProjectNodeSelectorParam])
157 157
 	}
158 158
 }
159 159
 
... ...
@@ -341,7 +341,7 @@ func CreateNewProject(clusterAdminClient *client.Client, clientConfig kclient.Co
341 341
 		AdminUser:   adminUser,
342 342
 	}
343 343
 
344
-	if err := newProjectOptions.Run(); err != nil {
344
+	if err := newProjectOptions.Run(false); err != nil {
345 345
 		return nil, err
346 346
 	}
347 347