From bc64713f990509067b303a945911709c45cde390 Mon Sep 17 00:00:00 2001 From: John Schnake Date: Mon, 9 Sep 2019 09:15:27 -0500 Subject: [PATCH] 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 --- cmd/sonobuoy/app/worker.go | 15 +++++++++++---- pkg/plugin/aggregation/handler.go | 15 ++++++++++++--- pkg/plugin/driver/base.go | 1 + pkg/plugin/driver/daemonset/daemonset.go | 6 +----- pkg/plugin/driver/job/job.go | 6 +----- 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/cmd/sonobuoy/app/worker.go b/cmd/sonobuoy/app/worker.go index 01aa77188..b65245262 100644 --- 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 index 04b5045fa..359c49701 100644 --- 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. `/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. `/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 index 6a627242a..fb4cefb07 100644 --- 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 index ab55643d2..757c7cf78 100644 --- 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 index 4fab5b28a..05b434bfc 100644 --- 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 {