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

Update 1.4 test to stop looking at field that isn't present. Fixes #… #37999

Merged
merged 1 commit into from
Dec 5, 2016
Merged

Update 1.4 test to stop looking at field that isn't present. Fixes #… #37999

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
14 changes: 9 additions & 5 deletions test/e2e/kubectl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,25 +498,29 @@ var _ = framework.KubeDescribe("Kubectl client", func() {
By("checking the result")
forEachReplicationController(c, ns, "app", "redis", validateReplicationControllerConfiguration)
})
It("should reuse nodePort when apply to an existing SVC", func() {
It("should reuse port when apply to an existing SVC", func() {
// This test doesn't do anything meaningful. It was supposed to test that the nodeport
Copy link
Member

Choose a reason for hiding this comment

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

if this test doesn't do anything meaningful, why do we have it?

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 is intended to do something meaningful - but was written in a way that didn't. I could just delete the whole test, but ideally we would fix it to test the right thing.

Copy link
Member

Choose a reason for hiding this comment

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

FYI This test is initially introduced by #24180 in v1.2

Copy link
Member Author

Choose a reason for hiding this comment

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

These changes were already made on master, and should have been backported to 1.4 branch, but weren't. This backports the changes to make the version skew test pass.

// did not get reset by apply, but never setup a nodeport typed service, so the
// nodeport was always 0. Updated to get version tests to pass after a change in 1.5
// to make kubectl jsonpath fail if the field is not present for default values
serviceJson := readTestFileOrDie(redisServiceFilename)
nsFlag := fmt.Sprintf("--namespace=%v", ns)

By("creating Redis SVC")
framework.RunKubectlOrDieInput(string(serviceJson[:]), "create", "-f", "-", nsFlag)

By("getting the original nodePort")
originalNodePort := framework.RunKubectlOrDie("get", "service", "redis-master", nsFlag, "-o", "jsonpath={.spec.ports[0].nodePort}")
originalPort := framework.RunKubectlOrDie("get", "service", "redis-master", nsFlag, "-o", "jsonpath={.spec.ports[0].port}")

By("applying the same configuration")
framework.RunKubectlOrDieInput(string(serviceJson[:]), "apply", "-f", "-", nsFlag)

By("getting the nodePort after applying configuration")
currentNodePort := framework.RunKubectlOrDie("get", "service", "redis-master", nsFlag, "-o", "jsonpath={.spec.ports[0].nodePort}")
currentPort := framework.RunKubectlOrDie("get", "service", "redis-master", nsFlag, "-o", "jsonpath={.spec.ports[0].port}")

By("checking the result")
if originalNodePort != currentNodePort {
framework.Failf("nodePort should keep the same")
if originalPort != currentPort {
framework.Failf("port should keep the same")
}
})
})
Expand Down