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.

Sign up for GitHub

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

Jump to bottom

avoid builds on non-linux nodes #22885

Merged
merged 1 commit into from May 23, 2019
Merged

avoid builds on non-linux nodes #22885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 17 additions & 2 deletions pkg/build/controller/build/defaults/defaults.go
View file
Open in desktop
@@ -1,6 +1,8 @@
package defaults

import (
"strings"

"k8s.io/klog"
"k8s.io/kubernetes/pkg/api/legacyscheme"

Expand Down Expand Up @@ -108,11 +110,24 @@ func (b BuildDefaults) applyPodProxyDefaults(pod *corev1.Pod, isCustomBuild bool
}

func (b BuildDefaults) applyPodDefaults(pod *corev1.Pod, isCustomBuild bool) {
if len(b.Config.NodeSelector) != 0 && pod.Spec.NodeSelector == nil {
nodeSelectorAppliable := pod.Spec.NodeSelector == nil
if !nodeSelectorAppliable && len(pod.Spec.NodeSelector) == 1 {
v, ok := pod.Spec.NodeSelector[corev1.LabelOSStable]
if ok && v == "linux" {
nodeSelectorAppliable = true
}
}
if len(b.Config.NodeSelector) != 0 && nodeSelectorAppliable {
// only apply nodeselector defaults if the pod has no nodeselector labels
// already.
pod.Spec.NodeSelector = map[string]string{}
if pod.Spec.NodeSelector == nil {
pod.Spec.NodeSelector = map[string]string{}
}
for k, v := range b.Config.NodeSelector {
// can't override kubernetes.io/os
if strings.TrimSpace(k) == corev1.LabelOSStable {
continue
}
addDefaultNodeSelector(k, v, pod.Spec.NodeSelector)
}
}
Expand Down
12 changes: 12 additions & 0 deletions pkg/build/controller/build/defaults/defaults_test.go
View file
Open in desktop
Expand Up @@ -516,6 +516,18 @@ func TestBuildDefaultsNodeSelector(t *testing.T) {
defaults: map[string]string{"key1": "default1", "key2": "default2"},
expected: map[string]string{"key1": "default1", "key2": "default2"},
},
{
name: "build - non empty linux node only",
build: testutil.Build().WithNodeSelector(map[string]string{corev1.LabelOSStable: "linux"}).AsBuild(),
defaults: map[string]string{"key1": "default1"},
expected: map[string]string{"key1": "default1", corev1.LabelOSStable: "linux"},
},
{
name: "build - try to change linux node only",
build: testutil.Build().WithNodeSelector(map[string]string{corev1.LabelOSStable: "linux"}).AsBuild(),
defaults: map[string]string{corev1.LabelOSStable: "windows"},
expected: map[string]string{corev1.LabelOSStable: "linux"},
},
{
name: "build - ignored",
build: testutil.Build().WithNodeSelector(map[string]string{"key1": "value1"}).AsBuild(),
Expand Down
4 changes: 4 additions & 0 deletions pkg/build/controller/build/overrides/overrides.go
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@ package overrides

import (
"fmt"
"strings"

"k8s.io/klog"

Expand Down Expand Up @@ -64,6 +65,9 @@ func (b BuildOverrides) ApplyOverrides(pod *corev1.Pod) error {
pod.Spec.NodeSelector = map[string]string{}
}
for k, v := range b.Config.NodeSelector {
if strings.TrimSpace(k) == corev1.LabelOSStable {
continue
}
klog.V(5).Infof("Adding override nodeselector %s=%s to build pod %s/%s", k, v, pod.Namespace, pod.Name)
pod.Spec.NodeSelector[k] = v
}
Expand Down
14 changes: 13 additions & 1 deletion pkg/build/controller/build/overrides/overrides_test.go
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@ import (
"reflect"
"testing"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apiserver/pkg/admission"

buildv1 "github.com/openshift/api/build/v1"
Expand Down Expand Up @@ -216,6 +216,18 @@ func TestBuildOverrideNodeSelector(t *testing.T) {
overrides: map[string]string{"key2": "override2"},
expected: map[string]string{"key1": "value1", "key2": "override2"},
},
{
name: "build - non empty linux node only",
build: testutil.Build().WithNodeSelector(map[string]string{v1.LabelOSStable: "linux"}).AsBuild(),
overrides: map[string]string{"key1": "default1"},
expected: map[string]string{"key1": "default1", v1.LabelOSStable: "linux"},
},
{
name: "build - try to change linux node only",
build: testutil.Build().WithNodeSelector(map[string]string{v1.LabelOSStable: "linux"}).AsBuild(),
overrides: map[string]string{v1.LabelOSStable: "windows"},
expected: map[string]string{v1.LabelOSStable: "linux"},
},
}

for _, test := range tests {
Expand Down
5 changes: 5 additions & 0 deletions pkg/build/controller/build/podcreationstrategy.go
View file
Open in desktop
Expand Up @@ -42,6 +42,11 @@ func (f *typeBasedFactoryStrategy) CreateBuildPod(build *buildv1.Build, addition
pod.Annotations = map[string]string{}
}
pod.Annotations[buildutil.BuildAnnotation] = build.Name

if pod.Spec.NodeSelector == nil {
pod.Spec.NodeSelector = map[string]string{}
}
pod.Spec.NodeSelector[corev1.LabelOSStable] = "linux"
}
return pod, err
}
6 changes: 5 additions & 1 deletion pkg/build/controller/build/podcreationstrategy_test.go
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@ import (
"fmt"
"testing"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"

buildv1 "github.com/openshift/api/build/v1"
)
Expand Down Expand Up @@ -111,5 +111,9 @@ func TestStrategyCreateBuildPod(t *testing.T) {
if pod != test.expectedPod {
t.Errorf("did not get expected pod with build %#v", test.build)
}
osType, _ := pod.Spec.NodeSelector[v1.LabelOSStable]
if osType != "linux" {
t.Errorf("did not get expected node selector in pod %#v", test.build)
}
}
}
41 changes: 33 additions & 8 deletions test/extended/builds/start.go
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ import (
g "github.com/onsi/ginkgo"
o "github.com/onsi/gomega"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
e2e "k8s.io/kubernetes/test/e2e/framework"
Expand All @@ -24,14 +25,21 @@ import (
var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
defer g.GinkgoRecover()
var (
buildFixture = exutil.FixturePath("testdata", "builds", "test-build.yaml")
bcWithPRRef = exutil.FixturePath("testdata", "builds", "test-bc-with-pr-ref.yaml")
exampleGemfile = exutil.FixturePath("testdata", "builds", "test-build-app", "Gemfile")
exampleBuild = exutil.FixturePath("testdata", "builds", "test-build-app")
symlinkFixture = exutil.FixturePath("testdata", "builds", "test-symlink-build.yaml")
exampleGemfileURL = "https://raw.githubusercontent.com/openshift/ruby-hello-world/master/Gemfile"
exampleArchiveURL = "https://github.com/openshift/ruby-hello-world/archive/master.zip"
oc = exutil.NewCLI("cli-start-build", exutil.KubeConfigPath())
buildFixture = exutil.FixturePath("testdata", "builds", "test-build.yaml")
bcWithPRRef = exutil.FixturePath("testdata", "builds", "test-bc-with-pr-ref.yaml")
exampleGemfile = exutil.FixturePath("testdata", "builds", "test-build-app", "Gemfile")
exampleBuild = exutil.FixturePath("testdata", "builds", "test-build-app")
symlinkFixture = exutil.FixturePath("testdata", "builds", "test-symlink-build.yaml")
exampleGemfileURL = "https://raw.githubusercontent.com/openshift/ruby-hello-world/master/Gemfile"
exampleArchiveURL = "https://github.com/openshift/ruby-hello-world/archive/master.zip"
oc = exutil.NewCLI("cli-start-build", exutil.KubeConfigPath())
verifyNodeSelector = func(oc *exutil.CLI, name string) {
pod, err := oc.KubeClient().CoreV1().Pods(oc.Namespace()).Get(name+"-build", metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred())
os, ok := pod.Spec.NodeSelector[corev1.LabelOSStable]
o.Expect(ok).To(o.BeTrue())
o.Expect(os).To(o.Equal("linux"))
}
)

g.Context("", func() {
Expand All @@ -57,6 +65,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
br, err := exutil.StartBuildAndWait(oc, "sample-build", "--wait")
o.Expect(err).NotTo(o.HaveOccurred())
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
})

g.It("should start a build and wait for the build to fail", func() {
Expand All @@ -65,6 +74,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
br.AssertFailure()
o.Expect(br.StartBuildErr).To(o.HaveOccurred()) // start-build should detect the build error with --wait flag
o.Expect(br.StartBuildStdErr).Should(o.ContainSubstring(`status is "Failed"`))
verifyNodeSelector(oc, br.BuildName)
})
})

Expand All @@ -82,9 +92,11 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
br, err := exutil.StartBuildAndWait(oc, "bc-with-pr-ref-docker")
o.Expect(err).NotTo(o.HaveOccurred())
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
br, err = exutil.StartBuildAndWait(oc, "bc-with-pr-ref")
o.Expect(err).NotTo(o.HaveOccurred())
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
out, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())

Expand All @@ -111,6 +123,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting the build with -e FOO=bar,VAR=test")
br, err := exutil.StartBuildAndWait(oc, "sample-build", "-e", "FOO=bar,VAR=test")
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())

Expand All @@ -127,6 +140,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting the build with buildconfig strategy env BUILD_LOGLEVEL=5")
br, err := exutil.StartBuildAndWait(oc, "sample-verbose-build")
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
g.By(fmt.Sprintf("verifying the build output is verbose"))
Expand All @@ -141,6 +155,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting the build with buildconfig strategy env BUILD_LOGLEVEL=5 but build-loglevel=1")
br, err := exutil.StartBuildAndWait(oc, "sample-verbose-build", "--build-loglevel=1")
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
g.By(fmt.Sprintf("verifying the build output is not verbose"))
Expand All @@ -163,6 +178,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting the build with a Dockerfile")
br, err := exutil.StartBuildAndWait(oc, "sample-build", fmt.Sprintf("--from-file=%s", exampleGemfile))
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
g.By(fmt.Sprintf("verifying the build %q status", br.BuildPath))
Expand All @@ -176,6 +192,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting the build with a directory")
br, err := exutil.StartBuildAndWait(oc, "sample-build", fmt.Sprintf("--from-dir=%s", exampleBuild))
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
g.By(fmt.Sprintf("verifying the build %q status", br.BuildPath))
Expand All @@ -189,6 +206,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
tryRepoInit(exampleBuild)
br, err := exutil.StartBuildAndWait(oc, "sample-build", fmt.Sprintf("--from-repo=%s", exampleBuild))
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())

Expand All @@ -208,6 +226,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
o.Expect(err).NotTo(o.HaveOccurred())
br, err := exutil.StartBuildAndWait(oc, "sample-build", fmt.Sprintf("--commit=%s", commit), fmt.Sprintf("--from-repo=%s", exampleBuild))
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())

Expand All @@ -223,6 +242,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting a valid build with a directory")
br, err := exutil.StartBuildAndWait(oc, "sample-build-binary", "--follow", fmt.Sprintf("--from-dir=%s", exampleBuild))
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(br.StartBuildStdErr).To(o.ContainSubstring("Uploading directory"))
Expand All @@ -244,6 +264,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting a valid build with input file served by https")
br, err := exutil.StartBuildAndWait(oc, "sample-build", fmt.Sprintf("--from-file=%s", exampleGemfileURL))
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(br.StartBuildStdErr).To(o.ContainSubstring(fmt.Sprintf("Uploading file from %q as binary input for the build", exampleGemfileURL)))
Expand All @@ -255,6 +276,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
// can't use sample-build-binary because we need contextDir due to github archives containing the top-level directory
br, err := exutil.StartBuildAndWait(oc, "sample-build-github-archive", fmt.Sprintf("--from-archive=%s", exampleArchiveURL))
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(br.StartBuildStdErr).To(o.ContainSubstring(fmt.Sprintf("Uploading archive from %q as binary input for the build", exampleArchiveURL)))
Expand Down Expand Up @@ -329,6 +351,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting the build without --build-arg flag")
br, _ := exutil.StartBuildAndWait(oc, "sample-build-docker-args-preset")
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
g.By("verifying the build output contains the build args from the BuildConfig.")
Expand All @@ -338,6 +361,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting the build with --build-arg flag")
br, _ := exutil.StartBuildAndWait(oc, "sample-build-docker-args", "--build-arg=foo=bar")
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
g.By("verifying the build output contains the changes.")
Expand All @@ -347,6 +371,7 @@ var _ = g.Describe("[Feature:Builds][Slow] starting a build using CLI", func() {
g.By("starting the build with --build-arg flag")
br, _ := exutil.StartBuildAndWait(oc, "sample-build-docker-args", "--build-arg=bar=foo")
br.AssertSuccess()
verifyNodeSelector(oc, br.BuildName)
buildLog, err := br.Logs()
o.Expect(err).NotTo(o.HaveOccurred())
g.By("verifying the build completed with a warning.")
Expand Down
4 changes: 2 additions & 2 deletions test/integration/buildpod_admission_test.go
View file
Open in desktop
Expand Up @@ -96,7 +96,7 @@ func TestBuildDefaultLabels(t *testing.T) {
}

func TestBuildDefaultNodeSelectors(t *testing.T) {
selectors := map[string]string{"KEY": "VALUE"}
selectors := map[string]string{"KEY": "VALUE", v1.LabelOSStable: "linux"}
oclient, kclientset, fn := setupBuildDefaultsAdmissionTest(t, &configapi.BuildDefaultsConfig{
NodeSelector: selectors,
})
Expand Down Expand Up @@ -205,7 +205,7 @@ func TestBuildOverrideLabels(t *testing.T) {
}

func TestBuildOverrideNodeSelectors(t *testing.T) {
selectors := map[string]string{"KEY": "VALUE"}
selectors := map[string]string{"KEY": "VALUE", v1.LabelOSStable: "linux"}
oclient, kclientset, fn := setupBuildOverridesAdmissionTest(t, &configapi.BuildOverridesConfig{
NodeSelector: selectors,
})
Expand Down