check if err is nil on metrics server run (#119)

* check if err is nil on metrics server run

* add documentation about running the tests

* add tests for metrics server start failure

* Specs: address linter issue

* Add script/test-unit information

---------

Co-authored-by: aram price <aramprice@users.noreply.github.com>
Co-authored-by: Ahmed Hassanin <ahmed.hassanin@sap.com>

Author: 3 people

README.md

@@ -18,3 +18,29 @@ This repository is a `GOPATH`. The [`.envrc`](.envrc) file provides a setup that
 Be careful with `go get`. In this repository `go get` will end up putting artifacts in the `src` directory, which you probably don't want to commit. It's impractical to `.gitignore` every possible package root in there so we have to apply discipline.
 
 To build a dev release, run a `bosh create-release` from this repo.
+
+## Running the tests
+
+Before the tests can be run, you need to execute the following commands:
+
+### On macOS
+
+```bash
+sudo ifconfig lo0 alias 127.0.0.2 up
+sudo ifconfig lo0 alias 127.0.0.3 up
+sudo ifconfig lo0 alias 127.0.0.253 up
+sudo ifconfig lo0 alias 127.0.0.254 up
+sudo sysctl -w kern.ipc.somaxconn=1024  # default is 128
+```
+
+Then run the tests:
+
+```bash
+./scripts/test-unit
+```
+
+You could also use ginkgo to run a specific test suite
+
+```bash
+ginkgo -v --procs=1 src/bosh-dns/dns/
+```

src/bosh-dns/dns/main.go

@@ -251,7 +251,9 @@ func mainExitCode() int {
 	if metricsServerWrapper != nil {
 		go func() {
 			err := metricsServerWrapper.Run(shutdown)
-			logger.Error(logTag, "could not start metric server: %s", err.Error())
+			if err != nil {
+				logger.Error(logTag, "could not start metrics server: %s", err.Error())
+			}
 		}()
 	}
 

src/bosh-dns/dns/main_test.go

@@ -1875,6 +1875,94 @@ var _ = Describe("main", func() {
 			Eventually(session.Out).Should(gbytes.Say(`[main].*ERROR - loading addresses configuration:.*addresses config file malformed:`))
 			Expect(session.Out.Contents()).To(ContainSubstring(fmt.Sprintf(`addresses config file malformed: %s`, addressesFile2.Name())))
 		})
+		Context("metrics server error scenarios", func() {
+			var (
+				session *gexec.Session
+			)
+
+			AfterEach(func() {
+				if session != nil {
+					session.Kill()
+					session.Wait()
+				}
+			})
+
+			It("logs error when metrics server fails to start due to port conflict", func() {
+				dnsPort, err := testhelpers.GetFreePort()
+				Expect(err).NotTo(HaveOccurred())
+
+				apiPort, err := testhelpers.GetFreePort()
+				Expect(err).NotTo(HaveOccurred())
+
+				metricsPort, err := testhelpers.GetFreePort()
+				Expect(err).NotTo(HaveOccurred())
+
+				// Create a conflict by binding to the metrics port first
+				conflictListener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", metricsPort))
+				Expect(err).NotTo(HaveOccurred())
+				defer conflictListener.Close() //nolint:errcheck
+
+				cfg := config.NewDefaultConfig()
+				cfg.Address = listenAddress
+				cfg.Port = dnsPort
+				cfg.JobsDir = jobsDir
+				cfg.Metrics = config.MetricsConfig{
+					Enabled: true,
+					Address: "127.0.0.1",
+					Port:    metricsPort, // This will conflict with our listener
+				}
+				cfg.API = config.APIConfig{
+					Port:            apiPort,
+					CAFile:          "api/assets/test_certs/test_ca.pem",
+					CertificateFile: "api/assets/test_certs/test_server.pem",
+					PrivateKeyFile:  "api/assets/test_certs/test_server.key",
+				}
+
+				cmd = newCommandWithConfig(cfg)
+				session, err = gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
+				Expect(err).NotTo(HaveOccurred())
+
+				// Wait for DNS server to start (so we know the process is running)
+				Expect(testhelpers.WaitForListeningTCP(dnsPort)).To(Succeed())
+
+				// Verify the error is logged
+				Eventually(session.Out, 10*time.Second).Should(
+					gbytes.Say(`\[main\].*ERROR.*could not start metrics server:`),
+				)
+			})
+
+			It("logs error when metrics server fails due to invalid address", func() {
+				dnsPort, err := testhelpers.GetFreePort()
+				Expect(err).NotTo(HaveOccurred())
+
+				apiPort, err := testhelpers.GetFreePort()
+				Expect(err).NotTo(HaveOccurred())
+
+				cfg := config.NewDefaultConfig()
+				cfg.Address = listenAddress
+				cfg.Port = dnsPort
+				cfg.JobsDir = jobsDir
+				cfg.Metrics = config.MetricsConfig{
+					Enabled: true,
+					Address: "invalid.address.that.does.not.exist",
+					Port:    8080,
+				}
+				cfg.API = config.APIConfig{
+					Port:            apiPort,
+					CAFile:          "api/assets/test_certs/test_ca.pem",
+					CertificateFile: "api/assets/test_certs/test_server.pem",
+					PrivateKeyFile:  "api/assets/test_certs/test_server.key",
+				}
+
+				cmd = newCommandWithConfig(cfg)
+				session, err = gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
+				Expect(err).NotTo(HaveOccurred())
+
+				Eventually(session.Out, 10*time.Second).Should(
+					gbytes.Say(`\[main\].*ERROR.*could not start metrics server:`),
+				)
+			})
+		})
 	})
 })