Adjust MASTER_URL to be just the hostname (#867)

In preparation of supporting status updates, I want to clarify the meaning of the MASTER_URL env var on the worker. Up until now the only URL of importance was the results URL so that is what the MASTER_URL was set to. Soon, we will have results and status updates, so it makes sense to have the MASTER_URL simply be the hostname and add the path on the fly depending on which endpoint we are trying to hit. ref #735 Signed-off-by: John Schnake <[email protected]>
vmware-tanzu · Sep 9, 2019 · bc64713 · bc64713
1 parent a603fdb
commit bc64713
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 17 deletions.
diff --git a/cmd/sonobuoy/app/worker.go b/cmd/sonobuoy/app/worker.go
@@ -21,14 +21,17 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"net/http"
+	"net/url"
 	"os"
 	"os/signal"
+	"path"
 	"strings"
 	"syscall"
 	"time"
 
 	"github.com/heptio/sonobuoy/pkg/errlog"
 	"github.com/heptio/sonobuoy/pkg/plugin"
+	"github.com/heptio/sonobuoy/pkg/plugin/aggregation"
 	"github.com/heptio/sonobuoy/pkg/worker"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -135,18 +138,22 @@ func runGather(global bool) error {
 		return errors.Wrap(err, "getting HTTP client")
 	}
 
-	url := ""
+	resultURL, err := url.Parse(cfg.MasterURL)
+	if err != nil {
+		return errors.Wrap(err, "parsing MasterURL")
+	}
+
 	if global {
 		// A global results URL looks like:
 		// http://sonobuoy-aggregator:8080/api/v1/results/global/systemd_logs
-		url = cfg.MasterURL + "/" + cfg.ResultType
+		resultURL.Path = path.Join(aggregation.PathResultsGlobal, cfg.ResultType)
 	} else {
 		// A single-node results URL looks like:
 		// http://sonobuoy-aggregator:8080/api/v1/results/by-node/node1/systemd_logs
-		url = cfg.MasterURL + "/" + cfg.NodeName + "/" + cfg.ResultType
+		resultURL.Path = path.Join(aggregation.PathResultsByNode, cfg.NodeName, cfg.ResultType)
 	}
 
-	err = worker.GatherResults(cfg.ResultsDir+"/done", url, client, sigHandler(plugin.GracefulShutdownPeriod*time.Second))
+	err = worker.GatherResults(cfg.ResultsDir+"/done", resultURL.String(), client, sigHandler(plugin.GracefulShutdownPeriod*time.Second))
 	return errors.Wrap(err, "gathering results")
 }
 

diff --git a/pkg/plugin/aggregation/handler.go b/pkg/plugin/aggregation/handler.go
@@ -32,10 +32,19 @@ const (
 	// have an /api/v2 later we'll figure out a good strategy for splitting up the
 	// handling.
 
+	// PathResultsByNode is the path for node-specific results to be PUT to. Callers should
+	// add two path elements as a suffix to this to specify the node and plugin (e.g. `<path>/node/plugin`)
+	PathResultsByNode = "/api/v1/results/by-node"
+
+	// PathResultsGlobal is the path for global (non-node-specific) results to be PUT to. Callers should
+	// add one path elements as a suffix to this to specify the plugin name (e.g. `<path>/plugin`)
+	PathResultsGlobal = "/api/v1/results/global"
+
 	// resultsGlobal is the path for node-specific results to be PUT
-	resultsByNode = "/api/v1/results/by-node/{node}/{plugin}"
-	// resultsGlobal is the path for global (non node-specific) results to be PUT
-	resultsGlobal = "/api/v1/results/global/{plugin}"
+	resultsByNode = PathResultsByNode + "/{node}/{plugin}"
+
+	// resultsGlobal is the path for global (non-node-specific) results to be PUT
+	resultsGlobal = PathResultsGlobal + "/{plugin}"
 
 	// defaultFilename is the name given to the file if no filename is given in the
 	// content-disposition header

diff --git a/pkg/plugin/driver/base.go b/pkg/plugin/driver/base.go
@@ -190,6 +190,7 @@ func (b *Base) workerEnvironment(hostname string, cert *tls.Certificate) []v1.En
 			},
 		},
 	}
+
 	return envVars
 }
 

diff --git a/pkg/plugin/driver/daemonset/daemonset.go b/pkg/plugin/driver/daemonset/daemonset.go
@@ -81,10 +81,6 @@ func (p *Plugin) ExpectedResults(nodes []v1.Node) []plugin.ExpectedResult {
 	return ret
 }
 
-func getAggregatorAddress(hostname string) string {
-	return fmt.Sprintf("https://%s/api/v1/results/by-node", hostname)
-}
-
 func (p *Plugin) createDaemonSetDefinition(hostname string, cert *tls.Certificate, ownerPod *v1.Pod) appsv1.DaemonSet {
 	ds := appsv1.DaemonSet{}
 	annotations := map[string]string{
@@ -159,7 +155,7 @@ func (p *Plugin) createDaemonSetDefinition(hostname string, cert *tls.Certificat
 
 // Run dispatches worker pods according to the DaemonSet's configuration.
 func (p *Plugin) Run(kubeclient kubernetes.Interface, hostname string, cert *tls.Certificate, ownerPod *v1.Pod) error {
-	daemonSet := p.createDaemonSetDefinition(getAggregatorAddress(hostname), cert, ownerPod)
+	daemonSet := p.createDaemonSetDefinition(fmt.Sprintf("https://%s", hostname), cert, ownerPod)
 
 	secret, err := p.MakeTLSSecret(cert)
 	if err != nil {

diff --git a/pkg/plugin/driver/job/job.go b/pkg/plugin/driver/job/job.go
@@ -74,10 +74,6 @@ func (p *Plugin) ExpectedResults(nodes []v1.Node) []plugin.ExpectedResult {
 	}
 }
 
-func getAggregatorAddress(hostname string) string {
-	return fmt.Sprintf("https://%s/api/v1/results/%v", hostname, plugin.GlobalResult)
-}
-
 func (p *Plugin) createPodDefinition(hostname string, cert *tls.Certificate, ownerPod *v1.Pod) v1.Pod {
 	pod := v1.Pod{}
 	annotations := map[string]string{
@@ -143,7 +139,7 @@ func (p *Plugin) createPodDefinition(hostname string, cert *tls.Certificate, own
 
 // Run dispatches worker pods according to the Job's configuration.
 func (p *Plugin) Run(kubeclient kubernetes.Interface, hostname string, cert *tls.Certificate, ownerPod *v1.Pod) error {
-	job := p.createPodDefinition(getAggregatorAddress(hostname), cert, ownerPod)
+	job := p.createPodDefinition(fmt.Sprintf("https://%s", hostname), cert, ownerPod)
 
 	secret, err := p.MakeTLSSecret(cert)
 	if err != nil {
-Original file line number
+Diff line change
@@ Expand Up @@
     			},
     		},
     	}
     	return envVars
     }
@@ Expand Down @@