Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions test/e2e/client_tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -735,15 +735,7 @@ func TestMTLSWithCRLs(t *testing.T) {
},
}

namespace := corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: namespaceName,
},
}
if err := kclient.Create(context.TODO(), &namespace); err != nil {
t.Fatalf("Failed to create namespace %q: %v", namespace.Name, err)
}
defer assertDeletedWaitForCleanup(t, kclient, &namespace)
namespace := createNamespace(t, namespaceName)
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
tcCerts := tc.CreateCerts()
Expand Down
18 changes: 1 addition & 17 deletions test/e2e/hsts_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ import (
configv1 "github.com/openshift/api/config/v1"
routev1 "github.com/openshift/api/route/v1"

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

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"

"k8s.io/utils/pointer"
Expand Down Expand Up @@ -72,20 +69,7 @@ func TestHstsPolicyWorks(t *testing.T) {
t.Logf("created a RequiredHSTSPolicy with DomainPatterns: %v,\n preload policy: %s,\n includeSubDomains policy: %s,\n largest age: %d,\n smallest age: %d\n", p.DomainPatterns, p.PreloadPolicy, p.IncludeSubDomainsPolicy, *p.MaxAge.LargestMaxAge, *p.MaxAge.SmallestMaxAge)

// Use the same namespace for route, service, and pod
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "hsts-policy-namespace",
},
}
if err := kclient.Create(context.TODO(), ns); err != nil {
t.Fatalf("failed to create namespace: %v", err)
}
defer func() {
// this will cleanup all components in this namespace
if err := kclient.Delete(context.TODO(), ns); err != nil {
t.Fatalf("failed to delete test namespace %s: %v", ns.Name, err)
}
}()
ns := createNamespace(t, "hsts-policy-namespace")

// Create pod
echoPod := buildEchoPod("hsts-policy-echo", ns.Name)
Expand Down
91 changes: 89 additions & 2 deletions test/e2e/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/utils/pointer"

"k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -55,7 +56,7 @@ func buildEchoPod(name, namespace string) *corev1.Pod {
`EXEC:'/bin/bash -c \"printf \\\"HTTP/1.0 200 OK\r\n\r\n\\\"; sed -e \\\"/^\r/q\\\"\"'`,
},
Command: []string{"/bin/socat"},
Image: "openshift/origin-node",
Image: "image-registry.openshift-image-registry.svc:5000/openshift/tools:latest",
Name: "echo",
Ports: []corev1.ContainerPort{
{
Expand Down Expand Up @@ -219,7 +220,7 @@ func buildSlowHTTPDPod(name, namespace string) *corev1.Pod {
`EXEC:'/bin/bash -c \"sleep 40; printf \\\"HTTP/1.0 200 OK\r\n\r\nfin\r\n\\\"\"'`,
},
Command: []string{"/bin/socat"},
Image: "openshift/origin-node",
Image: "image-registry.openshift-image-registry.svc:5000/openshift/tools:latest",
Name: "echo",
Ports: []corev1.ContainerPort{
{
Expand Down Expand Up @@ -688,3 +689,89 @@ func assertDeletedWaitForCleanup(t *testing.T, cl client.Client, thing client.Ob
t.Fatalf("Timed out waiting for %s to be cleaned up: %v", thing.GetName(), err)
}
}

// dumpEventsInNamespace gets the events in the specified namespace and logs
// them.
func dumpEventsInNamespace(t *testing.T, ns string) {
t.Helper()

eventList := &corev1.EventList{}
if err := kclient.List(context.TODO(), eventList, client.InNamespace(ns)); err != nil {
t.Errorf("failed to list events for namespace %s: %v", ns, err)
return
}

for _, e := range eventList.Items {
t.Log(e.FirstTimestamp, e.Source, e.InvolvedObject.Kind, e.InvolvedObject.Name, e.Reason, e.Message)
}
}

// createNamespace creates a namespace with the specified name and registers a
// cleanup handler to delete the namespace when the test finishes.
//
// After creating the namespace, this function waits for the "default"
// ServiceAccount and "system:image-pullers" RoleBinding to be created as well,
// which is necessary in order for pods in the new namespace to be able to pull
// images.
func createNamespace(t *testing.T, name string) *corev1.Namespace {
t.Helper()

t.Logf("Creating namespace %q...", name)
ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: name}}
if err := kclient.Create(context.TODO(), ns); err != nil {
t.Fatalf("failed to create namespace: %v", err)
}
t.Cleanup(func() {
t.Logf("Dumping events in namespace %q...", name)
Copy link
Contributor

Choose a reason for hiding this comment

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

I hadn't noticed this earlier, but this log message belongs inside the if t.Failed() condition brackets. I don't think it needs to fixed in the other PRs, but do you want to fix it here and in the 4.11 backports while they are open?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hadn't noticed this earlier, but this log message belongs inside the if t.Failed() condition brackets.

According to https://pkg.go.dev/testing#T.Log:

For tests, the text will be printed only if the test fails or the -test.v flag is set.

So I don't believe there's a need to add if t.Failed().

I don't think it needs to fixed in the other PRs, but do you want to fix it here and in the 4.11 backports while they are open?

Backports should be as close as possible to the original changes (though resolving conflicts sometimes necessitates some deviation). Otherwise it gets more difficult to keep track of what actually got backported to which version, and subsequent backports are more likely to encounter conflicts. If we need additional changes to this code, I'd rather open a new Jira issue, make the additional change in the main branch, and do another series of backports if necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll leave that up to you, it's not a big deal.

if t.Failed() {
dumpEventsInNamespace(t, name)
}
t.Logf("Deleting namespace %q...", name)
if err := kclient.Delete(context.TODO(), ns); err != nil {
t.Errorf("failed to delete namespace %s: %v", ns.Name, err)
}
})

saName := types.NamespacedName{
Namespace: name,
Name: "default",
}
t.Logf("Waiting for ServiceAccount %s to be provisioned...", saName)
if err := wait.PollImmediate(1*time.Second, 3*time.Minute, func() (bool, error) {
var sa corev1.ServiceAccount
if err := kclient.Get(context.TODO(), saName, &sa); err != nil {
if errors.IsNotFound(err) {
return false, nil
}
return false, err
}
for _, s := range sa.Secrets {
if strings.Contains(s.Name, "dockercfg") {
return true, nil
}
}
return false, nil
}); err != nil {
t.Fatalf(`Timed out waiting for ServiceAccount %s to be provisioned: %v`, saName, err)
}

rbName := types.NamespacedName{
Namespace: name,
Name: "system:image-pullers",
}
t.Logf("Waiting for RoleBinding %s to be created...", rbName)
if err := wait.PollImmediate(1*time.Second, 3*time.Minute, func() (bool, error) {
var rb rbacv1.RoleBinding
if err := kclient.Get(context.TODO(), rbName, &rb); err != nil {
if errors.IsNotFound(err) {
return false, nil
}
return false, err
}
return true, nil
}); err != nil {
t.Fatalf(`Timed out waiting for RoleBinding "default" to be provisioned: %v`, err)
}

return ns
}