Skip to content

Commit e0a1e98

Browse files
sjbermansjberman
committed
Test code review
1 parent c1552b6 commit e0a1e98

File tree

5 files changed

+187
-211
lines changed

5 files changed

+187
-211
lines changed

‎tests/framework/collector.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package framework
2+
3+
import (
4+
"fmt"
5+
"os/exec"
6+
7+
"sigs.k8s.io/controller-runtime/pkg/client"
8+
)
9+
10+
const (
11+
CollectorNamespace = "collector"
12+
collectorChartReleaseName = "otel-collector"
13+
// FIXME(pleshakov): Find a automated way to keep the version updated here similar to dependabot.
14+
// https://github.com/nginxinc/nginx-gateway-fabric/issues/1665
15+
collectorChartVersion = "0.73.1"
16+
)
17+
18+
// InstallCollector installs the otel-collector.
19+
func InstallCollector() ([]byte, error) {
20+
repoAddArgs := []string{
21+
"repo",
22+
"add",
23+
"open-telemetry",
24+
"https://open-telemetry.github.io/opentelemetry-helm-charts",
25+
}
26+
27+
if output, err := exec.Command("helm", repoAddArgs...).CombinedOutput(); err != nil {
28+
return output, err
29+
}
30+
31+
args := []string{
32+
"install",
33+
collectorChartReleaseName,
34+
"open-telemetry/opentelemetry-collector",
35+
"--create-namespace",
36+
"--namespace", CollectorNamespace,
37+
"--version", collectorChartVersion,
38+
"-f", "manifests/telemetry/collector-values.yaml",
39+
"--wait",
40+
}
41+
42+
return exec.Command("helm", args...).CombinedOutput()
43+
}
44+
45+
// UninstallCollector uninstalls the otel-collector.
46+
func UninstallCollector(resourceManager ResourceManager) ([]byte, error) {
47+
args := []string{
48+
"uninstall", collectorChartReleaseName,
49+
"--namespace", CollectorNamespace,
50+
}
51+
52+
output, err := exec.Command("helm", args...).CombinedOutput()
53+
if err != nil {
54+
return output, err
55+
}
56+
57+
return nil, resourceManager.DeleteNamespace(CollectorNamespace)
58+
}
59+
60+
// GetCollectorPodName returns the name of the collector Pod.
61+
func GetCollectorPodName(resourceManager ResourceManager) (string, error) {
62+
collectorPodNames, err := resourceManager.GetPodNames(
63+
CollectorNamespace,
64+
client.MatchingLabels{
65+
"app.kubernetes.io/name": "opentelemetry-collector",
66+
},
67+
)
68+
if err != nil {
69+
return "", err
70+
}
71+
72+
if len(collectorPodNames) != 1 {
73+
return "", fmt.Errorf("expected 1 collector pod, got %d", len(collectorPodNames))
74+
}
75+
76+
return collectorPodNames[0], nil
77+
}

‎tests/framework/request.go

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,10 @@ import (
66
"crypto/tls"
77
"fmt"
88
"io"
9-
"log"
109
"net"
1110
"net/http"
1211
"strings"
1312
"time"
14-
15-
"k8s.io/apimachinery/pkg/util/wait"
1613
)
1714

1815
// Get sends a GET request to the specified url.
@@ -54,7 +51,7 @@ func makeRequest(method, url, address string, body io.Reader, timeout time.Durat
5451
return dialer.DialContext(ctx, network, fmt.Sprintf("%s:%s", address, port))
5552
}
5653

57-
ctx, cancel := context.WithTimeout(context.Background(), requestTimeout)
54+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
5855
defer cancel()
5956

6057
req, err := http.NewRequestWithContext(ctx, method, url, body)
@@ -80,36 +77,3 @@ func makeRequest(method, url, address string, body io.Reader, timeout time.Durat
8077

8178
return resp, nil
8279
}
83-
84-
// GetWithRetry retries the Get function until it succeeds or the context times out.
85-
func GetWithRetry(
86-
ctx context.Context,
87-
url,
88-
address string,
89-
requestTimeout time.Duration,
90-
) (int, string, error) {
91-
var statusCode int
92-
var body string
93-
94-
err := wait.PollUntilContextCancel(
95-
ctx,
96-
500*time.Millisecond,
97-
true, /* poll immediately */
98-
func(ctx context.Context) (bool, error) {
99-
var getErr error
100-
statusCode, body, getErr = Get(url, address, requestTimeout)
101-
if getErr != nil {
102-
return false, getErr
103-
}
104-
105-
if statusCode != 200 {
106-
log.Printf("got %d code instead of expected 200\n", statusCode)
107-
return false, nil
108-
}
109-
110-
return true, nil
111-
},
112-
)
113-
114-
return statusCode, body, err
115-
}

‎tests/suite/sample_test.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package suite
22

33
import (
4-
"context"
54
"fmt"
65
"net/http"
76
"strconv"
7+
"strings"
88
"time"
99

1010
. "github.com/onsi/ginkgo/v2"
@@ -48,12 +48,23 @@ var _ = Describe("Basic test example", Label("functional"), func() {
4848
url = fmt.Sprintf("http://foo.example.com:%s/hello", strconv.Itoa(portFwdPort))
4949
}
5050

51-
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
52-
defer cancel()
53-
54-
status, body, err := framework.GetWithRetry(ctx, url, address, timeoutConfig.RequestTimeout)
55-
Expect(err).ToNot(HaveOccurred())
56-
Expect(status).To(Equal(http.StatusOK))
57-
Expect(body).To(ContainSubstring("URI: /hello"))
51+
Eventually(
52+
func() error {
53+
status, body, err := framework.Get(url, address, timeoutConfig.RequestTimeout)
54+
if err != nil {
55+
return err
56+
}
57+
if status != http.StatusOK {
58+
return fmt.Errorf("status not 200; got %d", status)
59+
}
60+
expBody := "URI: /hello"
61+
if !strings.Contains(body, expBody) {
62+
return fmt.Errorf("bad body: got %s; expected %s", body, expBody)
63+
}
64+
return nil
65+
}).
66+
WithTimeout(timeoutConfig.RequestTimeout).
67+
WithPolling(500 * time.Millisecond).
68+
Should(Succeed())
5869
})
5970
})

‎tests/suite/telemetry_test.go

Lines changed: 6 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,22 @@
11
package suite
22

33
import (
4-
"context"
54
"fmt"
6-
"os/exec"
75
"strings"
8-
"time"
96

107
. "github.com/onsi/ginkgo/v2"
118
. "github.com/onsi/gomega"
129
core "k8s.io/api/core/v1"
13-
apierrors "k8s.io/apimachinery/pkg/api/errors"
14-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15-
crClient "sigs.k8s.io/controller-runtime/pkg/client"
16-
)
1710

18-
const (
19-
collectorNamespace = "collector"
20-
collectorChartReleaseName = "otel-collector"
21-
// FIXME(pleshakov): Find a automated way to keep the version updated here similar to dependabot.
22-
// https://github.com/nginxinc/nginx-gateway-fabric/issues/1665
23-
collectorChartVersion = "0.73.1"
11+
"github.com/nginxinc/nginx-gateway-fabric/tests/framework"
2412
)
2513

2614
var _ = Describe("Telemetry test with OTel collector", Label("telemetry"), func() {
2715
BeforeEach(func() {
2816
// Because NGF reports telemetry on start, we need to install the collector first.
2917

3018
// Install collector
31-
output, err := installCollector()
19+
output, err := framework.InstallCollector()
3220
Expect(err).ToNot(HaveOccurred(), string(output))
3321

3422
// Install NGF
@@ -41,22 +29,13 @@ var _ = Describe("Telemetry test with OTel collector", Label("telemetry"), func(
4129
})
4230

4331
AfterEach(func() {
44-
output, err := uninstallCollector()
32+
output, err := framework.UninstallCollector(resourceManager)
4533
Expect(err).ToNot(HaveOccurred(), string(output))
4634
})
4735

4836
It("sends telemetry", func() {
49-
names, err := resourceManager.GetPodNames(
50-
collectorNamespace,
51-
crClient.MatchingLabels{
52-
"app.kubernetes.io/name": "opentelemetry-collector",
53-
},
54-
)
55-
37+
name, err := framework.GetCollectorPodName(resourceManager)
5638
Expect(err).ToNot(HaveOccurred())
57-
Expect(names).To(HaveLen(1))
58-
59-
name := names[0]
6039

6140
// We assert that all data points were sent
6241
// For some data points, as a sanity check, we assert on sent values.
@@ -68,15 +47,15 @@ var _ = Describe("Telemetry test with OTel collector", Label("telemetry"), func(
6847
Expect(err).ToNot(HaveOccurred())
6948

7049
matchFirstExpectedLine := func() bool {
71-
logs, err := resourceManager.GetPodLogs(collectorNamespace, name, &core.PodLogOptions{})
50+
logs, err := resourceManager.GetPodLogs(framework.CollectorNamespace, name, &core.PodLogOptions{})
7251
Expect(err).ToNot(HaveOccurred())
7352
return strings.Contains(logs, "dataType: Str(ngf-product-telemetry)")
7453
}
7554

7655
// Wait until the collector has received the telemetry data
7756
Eventually(matchFirstExpectedLine, "30s", "5s").Should(BeTrue())
7857

79-
logs, err := resourceManager.GetPodLogs(collectorNamespace, name, &core.PodLogOptions{})
58+
logs, err := resourceManager.GetPodLogs(framework.CollectorNamespace, name, &core.PodLogOptions{})
8059
Expect(err).ToNot(HaveOccurred())
8160

8261
assertConsecutiveLinesInLogs(
@@ -107,54 +86,6 @@ var _ = Describe("Telemetry test with OTel collector", Label("telemetry"), func(
10786
})
10887
})
10988

110-
func installCollector() ([]byte, error) {
111-
repoAddArgs := []string{
112-
"repo",
113-
"add",
114-
"open-telemetry",
115-
"https://open-telemetry.github.io/opentelemetry-helm-charts",
116-
}
117-
118-
if output, err := exec.Command("helm", repoAddArgs...).CombinedOutput(); err != nil {
119-
return output, err
120-
}
121-
122-
args := []string{
123-
"install",
124-
collectorChartReleaseName,
125-
"open-telemetry/opentelemetry-collector",
126-
"--create-namespace",
127-
"--namespace", collectorNamespace,
128-
"--version", collectorChartVersion,
129-
"-f", "manifests/telemetry/collector-values.yaml",
130-
"--wait",
131-
}
132-
133-
return exec.Command("helm", args...).CombinedOutput()
134-
}
135-
136-
func uninstallCollector() ([]byte, error) {
137-
args := []string{
138-
"uninstall", collectorChartReleaseName,
139-
"--namespace", collectorNamespace,
140-
}
141-
142-
output, err := exec.Command("helm", args...).CombinedOutput()
143-
if err != nil {
144-
return output, err
145-
}
146-
147-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
148-
defer cancel()
149-
150-
err = k8sClient.Delete(ctx, &core.Namespace{ObjectMeta: metav1.ObjectMeta{Name: collectorNamespace}})
151-
if err != nil && !apierrors.IsNotFound(err) {
152-
return nil, err
153-
}
154-
155-
return nil, resourceManager.DeleteNamespace(collectorNamespace)
156-
}
157-
15889
func assertConsecutiveLinesInLogs(logs string, expectedLines []string) {
15990
lines := strings.Split(logs, "\n")
16091

0 commit comments

Comments
 (0)