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

test: Set num-nodes flag #16176

Merged
merged 1 commit into from
Jan 20, 2024
Merged

test: Set num-nodes flag #16176

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion tests/e2e/pkg/tester/skip_regex.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
)

const (
skipRegexBase = "\\[Slow\\]|\\[Serial\\]|\\[Disruptive\\]|\\[Flaky\\]|\\[Feature:.+\\]|nfs|NFS|Gluster"
skipRegexBase = "\\[Slow\\]|\\[Serial\\]|\\[Disruptive\\]|\\[Flaky\\]|\\[Feature:.+\\]|nfs|NFS|Gluster|NodeProblemDetector"
Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to adjust the prow job generator and it caused a big mess. We run ubuntu by default so it would need to be running most of the time. It is easier to skip, will open a PR in k/k now to park this behind a Feature tag

)

func (t *Tester) setSkipRegexFlag() error {
Expand Down Expand Up @@ -81,6 +81,8 @@ func (t *Tester) setSkipRegexFlag() error {
skipRegex += "|same.hostPort.but.different.hostIP.and.protocol"
// https://github.com/cilium/cilium/issues/9207
skipRegex += "|serve.endpoints.on.same.port.and.different.protocols"
// https://github.com/kubernetes/kubernetes/blob/418ae605ec1b788d43bff7ac44af66d8b669b833/test/e2e/network/networking.go#L135
skipRegex += "|should.check.kube-proxy.urls"

if isPre28 {
// These may be fixed in Cilium 1.13 but skipping for now
Expand Down
15 changes: 10 additions & 5 deletions tests/e2e/pkg/tester/tester.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"os"
"os/exec"
"strconv"
"strings"

"github.com/octago/sflags/gen/gpflag"
Expand Down Expand Up @@ -187,11 +188,6 @@ func (t *Tester) addNodeIG() error {
return err
}

// Skip this function for non gce clusters
if cluster.Spec.LegacyCloudProvider != "gce" {
return nil
}

igs, err := kops.GetInstanceGroups("kops", cluster.Name, nil)
if err != nil {
return err
Expand All @@ -203,6 +199,15 @@ func (t *Tester) addNodeIG() error {
ig = v
}
}
numNodes := int(*ig.Spec.MaxSize) // we assume that MinSize = Maxsize, this is true for e2e testing
Copy link
Member

Choose a reason for hiding this comment

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

This will be the size of the last nodes InstanceGroup. Should we instead sum the max size of all nodes InstanceGroups?

Copy link
Member Author

@upodroid upodroid Dec 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

No, the ginkgo wants the number of all nodes except the master/controplane IG. When we are using kops for e2e testing we assume there are exactly 2 IGs(one master and one worker). If we add the master ig to the count, you'll have an extra node that you can't schedule workloads onto without tweaking manifests to tolerate control plane taints

Copy link
Member

Choose a reason for hiding this comment

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

Ah I was thinking in the case of there being multiple nodes IGs. This tester code can be used in cluster configurations that have multiple node IGs so I don't know if your assumption is always true.

klog.Infof("Setting -num-nodes=%v", numNodes)
t.TestArgs += " -num-nodes=" + strconv.Itoa(numNodes)

// Skip the rest of this function for non gce clusters
if cluster.Spec.LegacyCloudProvider != "gce" {
return nil
}

nodeTag := gce.TagForRole(cluster.ObjectMeta.Name, unversioned.InstanceGroupRoleNode)
klog.Infof("Setting --node-tag=%s", nodeTag)
t.TestArgs += " --node-tag=" + nodeTag
Expand Down
Loading