From cc9db47f6834eb7f6bde84a1ba692454d75c816b Mon Sep 17 00:00:00 2001 From: Joseph Phillips Date: Mon, 4 Mar 2024 13:33:54 +0100 Subject: [PATCH 1/2] Adds agent as an input to the controller agent config worker. This is used only to get the current agent ID, which will be used by the controller charm to set config in the correct path for the agent, which may not have the same ID as the controller unit. Missing inputs are also populated for the same mode manifolds. --- .../agent/machine/manifolds.go | 1 + .../agent/safemode/manifolds.go | 8 +++- internal/socketlistener/package_test.go | 2 +- .../worker/controlleragentconfig/manifold.go | 18 +++++++- .../controlleragentconfig/manifold_test.go | 45 +++++++++++++++++-- .../worker/controlleragentconfig/worker.go | 4 ++ 6 files changed, 69 insertions(+), 9 deletions(-) diff --git a/cmd/jujud-controller/agent/machine/manifolds.go b/cmd/jujud-controller/agent/machine/manifolds.go index efe1a923492..785b37737b6 100644 --- a/cmd/jujud-controller/agent/machine/manifolds.go +++ b/cmd/jujud-controller/agent/machine/manifolds.go @@ -360,6 +360,7 @@ func commonManifolds(config ManifoldsConfig) dependency.Manifolds { // Controller agent config manifold watches the controller // agent config and bounces if it changes. controllerAgentConfigName: ifController(controlleragentconfig.Manifold(controlleragentconfig.ManifoldConfig{ + AgentName: agentName, Clock: config.Clock, Logger: loggo.GetLogger("juju.worker.controlleragentconfig"), NewSocketListener: controlleragentconfig.NewSocketListener, diff --git a/cmd/jujud-controller/agent/safemode/manifolds.go b/cmd/jujud-controller/agent/safemode/manifolds.go index e8cc8a1255c..fc057d7c517 100644 --- a/cmd/jujud-controller/agent/safemode/manifolds.go +++ b/cmd/jujud-controller/agent/safemode/manifolds.go @@ -17,6 +17,7 @@ import ( coreagent "github.com/juju/juju/agent" "github.com/juju/juju/agent/engine" "github.com/juju/juju/cmd/jujud-controller/util" + "github.com/juju/juju/core/paths" "github.com/juju/juju/internal/worker/agent" "github.com/juju/juju/internal/worker/controlleragentconfig" "github.com/juju/juju/internal/worker/dbaccessor" @@ -99,8 +100,11 @@ func commonManifolds(config ManifoldsConfig) dependency.Manifolds { // Controller agent config manifold watches the controller // agent config and bounces if it changes. controllerAgentConfigName: ifController(controlleragentconfig.Manifold(controlleragentconfig.ManifoldConfig{ - Clock: config.Clock, - Logger: loggo.GetLogger("juju.worker.controlleragentconfig"), + AgentName: agentName, + Clock: config.Clock, + Logger: loggo.GetLogger("juju.worker.controlleragentconfig"), + NewSocketListener: controlleragentconfig.NewSocketListener, + SocketName: paths.ConfigChangeSocket(paths.OSUnixLike), })), // The stateconfigwatcher manifold watches the machine agent's diff --git a/internal/socketlistener/package_test.go b/internal/socketlistener/package_test.go index cf07f24e464..b4cc437daad 100644 --- a/internal/socketlistener/package_test.go +++ b/internal/socketlistener/package_test.go @@ -1,7 +1,7 @@ // Copyright 2024 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. -package socketlistener_test +package socketlistener import ( "testing" diff --git a/internal/worker/controlleragentconfig/manifold.go b/internal/worker/controlleragentconfig/manifold.go index 077d16b7936..c49d986fd83 100644 --- a/internal/worker/controlleragentconfig/manifold.go +++ b/internal/worker/controlleragentconfig/manifold.go @@ -11,6 +11,7 @@ import ( "github.com/juju/worker/v4" "github.com/juju/worker/v4/dependency" + "github.com/juju/juju/agent" "github.com/juju/juju/internal/socketlistener" ) @@ -35,8 +36,9 @@ type Logger interface { // ManifoldConfig defines the configuration for the agent controller config // manifold. type ManifoldConfig struct { - Logger Logger - Clock clock.Clock + AgentName string + Logger Logger + Clock clock.Clock // SocketName is the socket file descriptor. SocketName string // NewSocketListener is the function that creates a new socket listener. @@ -45,6 +47,9 @@ type ManifoldConfig struct { // Validate validates the manifold configuration. func (cfg ManifoldConfig) Validate() error { + if cfg.AgentName == "" { + return errors.NotValidf("empty AgentName") + } if cfg.Logger == nil { return errors.NotValidf("nil Logger") } @@ -63,13 +68,22 @@ func (cfg ManifoldConfig) Validate() error { // Manifold returns a dependency manifold that runs the trace worker. func Manifold(config ManifoldConfig) dependency.Manifold { return dependency.Manifold{ + Inputs: []string{ + config.AgentName, + }, Output: configOutput, Start: func(ctx context.Context, getter dependency.Getter) (worker.Worker, error) { if err := config.Validate(); err != nil { return nil, errors.Trace(err) } + var thisAgent agent.Agent + if err := getter.Get(config.AgentName, &thisAgent); err != nil { + return nil, errors.Trace(err) + } + w, err := NewWorker(WorkerConfig{ + ControllerID: thisAgent.CurrentConfig().Tag().Id(), Logger: config.Logger, Clock: config.Clock, NewSocketListener: config.NewSocketListener, diff --git a/internal/worker/controlleragentconfig/manifold_test.go b/internal/worker/controlleragentconfig/manifold_test.go index 5b2f023af17..f508697de46 100644 --- a/internal/worker/controlleragentconfig/manifold_test.go +++ b/internal/worker/controlleragentconfig/manifold_test.go @@ -5,6 +5,8 @@ package controlleragentconfig import ( "context" + "github.com/juju/juju/agent" + "github.com/juju/names/v5" "github.com/juju/clock" "github.com/juju/errors" @@ -17,16 +19,29 @@ import ( type manifoldSuite struct { baseSuite + + agent *mockAgent } var _ = gc.Suite(&manifoldSuite{}) +func (s *manifoldSuite) SetUpTest(c *gc.C) { + s.baseSuite.SetUpTest(c) + + s.agent = new(mockAgent) + s.agent.conf.tag = names.NewMachineTag("99") +} + func (s *manifoldSuite) TestValidateConfig(c *gc.C) { defer s.setupMocks(c).Finish() cfg := s.getConfig() c.Check(cfg.Validate(), jc.ErrorIsNil) + cfg = s.getConfig() + cfg.AgentName = "" + c.Check(cfg.Validate(), jc.ErrorIs, errors.NotValid) + cfg = s.getConfig() cfg.Logger = nil c.Check(cfg.Validate(), jc.ErrorIs, errors.NotValid) @@ -46,6 +61,7 @@ func (s *manifoldSuite) TestValidateConfig(c *gc.C) { func (s *manifoldSuite) getConfig() ManifoldConfig { return ManifoldConfig{ + AgentName: "agent", Logger: s.logger, Clock: clock.WallClock, NewSocketListener: NewSocketListener, @@ -54,14 +70,14 @@ func (s *manifoldSuite) getConfig() ManifoldConfig { } func (s *manifoldSuite) newContext() dependency.Getter { - resources := map[string]any{} + resources := map[string]any{ + "agent": s.agent, + } return dependencytesting.StubGetter(resources) } -var expectedInputs = []string{} - func (s *manifoldSuite) TestInputs(c *gc.C) { - c.Assert(Manifold(s.getConfig()).Inputs, jc.SameContents, expectedInputs) + c.Assert(Manifold(s.getConfig()).Inputs, jc.SameContents, []string{"agent"}) } func (s *manifoldSuite) TestStart(c *gc.C) { @@ -84,3 +100,24 @@ func (s *manifoldSuite) TestOutput(c *gc.C) { c.Assert(man.Output(w, &watcher), jc.ErrorIsNil) c.Assert(watcher, gc.NotNil) } + +type mockAgent struct { + agent.Agent + conf mockConfig +} + +func (ma *mockAgent) CurrentConfig() agent.Config { + return &ma.conf +} + +type mockConfig struct { + agent.ConfigSetter + tag names.Tag +} + +func (mc *mockConfig) Tag() names.Tag { + if mc.tag == nil { + return names.NewMachineTag("99") + } + return mc.tag +} diff --git a/internal/worker/controlleragentconfig/worker.go b/internal/worker/controlleragentconfig/worker.go index fbf3618770c..ed10ee37fea 100644 --- a/internal/worker/controlleragentconfig/worker.go +++ b/internal/worker/controlleragentconfig/worker.go @@ -31,6 +31,9 @@ const ( // WorkerConfig encapsulates the configuration options for the // agent controller config worker. type WorkerConfig struct { + // ControllerID is the ID of this controller. + ControllerID string + // Logger writes log messages. Logger Logger // Clock is needed for worker.NewRunner. Clock clock.Clock @@ -136,6 +139,7 @@ func newWorker(cfg WorkerConfig, internalStates chan string) (*configWorker, err func (w *configWorker) registerHandlers(r *mux.Router) { r.HandleFunc("/reload", w.reloadHandler). Methods(http.MethodPost) + } // reloadHandler sends a signal to the configWorker when a config reload is From c4b0bf3ad2a5dec8e51273dfb93724e1fca04941 Mon Sep 17 00:00:00 2001 From: Joseph Phillips Date: Mon, 4 Mar 2024 14:10:50 +0100 Subject: [PATCH 2/2] Adds path to controller agent socket that return the agent's configured ID. --- .../controlleragentconfig/manifold_test.go | 5 +-- .../worker/controlleragentconfig/worker.go | 14 +++++++- .../controlleragentconfig/worker_test.go | 35 +++++++++++++++---- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/internal/worker/controlleragentconfig/manifold_test.go b/internal/worker/controlleragentconfig/manifold_test.go index f508697de46..7a4d38dfb01 100644 --- a/internal/worker/controlleragentconfig/manifold_test.go +++ b/internal/worker/controlleragentconfig/manifold_test.go @@ -5,16 +5,17 @@ package controlleragentconfig import ( "context" - "github.com/juju/juju/agent" - "github.com/juju/names/v5" "github.com/juju/clock" "github.com/juju/errors" + "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" "github.com/juju/worker/v4/dependency" dependencytesting "github.com/juju/worker/v4/dependency/testing" "github.com/juju/worker/v4/workertest" gc "gopkg.in/check.v1" + + "github.com/juju/juju/agent" ) type manifoldSuite struct { diff --git a/internal/worker/controlleragentconfig/worker.go b/internal/worker/controlleragentconfig/worker.go index ed10ee37fea..dd23ec7636f 100644 --- a/internal/worker/controlleragentconfig/worker.go +++ b/internal/worker/controlleragentconfig/worker.go @@ -139,7 +139,8 @@ func newWorker(cfg WorkerConfig, internalStates chan string) (*configWorker, err func (w *configWorker) registerHandlers(r *mux.Router) { r.HandleFunc("/reload", w.reloadHandler). Methods(http.MethodPost) - + r.HandleFunc("/agent-id", w.idHandler). + Methods(http.MethodGet) } // reloadHandler sends a signal to the configWorker when a config reload is @@ -155,6 +156,17 @@ func (w *configWorker) reloadHandler(resp http.ResponseWriter, req *http.Request } } +// idHandler simply returns this agent's ID. +// It is used by the *unit* to get the *controller's* ID. +func (w *configWorker) idHandler(resp http.ResponseWriter, req *http.Request) { + resp.Header().Set("Content-Type", "application/text") + + _, err := resp.Write([]byte(w.cfg.ControllerID)) + if err != nil { + w.cfg.Logger.Errorf("error writing HTTP response: %v", err) + } +} + // Kill is part of the worker.Worker interface. func (w *configWorker) Kill() { w.catacomb.Kill(nil) diff --git a/internal/worker/controlleragentconfig/worker_test.go b/internal/worker/controlleragentconfig/worker_test.go index 2dceeda9e6a..04c826981b2 100644 --- a/internal/worker/controlleragentconfig/worker_test.go +++ b/internal/worker/controlleragentconfig/worker_test.go @@ -5,6 +5,7 @@ package controlleragentconfig import ( "context" + "io" "net" "net/http" "path" @@ -38,6 +39,27 @@ func (s *workerSuite) TestStartup(c *gc.C) { workertest.CleanKill(c, w) } +func (s *workerSuite) TestIDRequest(c *gc.C) { + defer s.setupMocks(c).Finish() + + w, socket, states := s.newWorker(c) + defer workertest.DirtyKill(c, w) + + s.ensureStartup(c, states) + + resp, err := newRequest(c, socket, "/agent-id", http.MethodGet) + c.Assert(err, jc.ErrorIsNil) + defer func() { _ = resp.Body.Close() }() + + c.Assert(resp.StatusCode, gc.Equals, http.StatusOK) + + content, err := io.ReadAll(resp.Body) + c.Assert(err, jc.ErrorIsNil) + c.Check(string(content), gc.Equals, "99") + + workertest.CleanKill(c, w) +} + func (s *workerSuite) TestReloadRequest(c *gc.C) { defer s.setupMocks(c).Finish() @@ -342,6 +364,7 @@ func (s *workerSuite) newWorker(c *gc.C) (*configWorker, string, chan string) { socket := path.Join(tmpDir, "test.socket") w, err := newWorker(WorkerConfig{ + ControllerID: "99", Logger: s.logger, Clock: clock.WallClock, SocketName: socket, @@ -370,18 +393,18 @@ func (s *workerSuite) ensureReload(c *gc.C, states chan string) { } func (s *workerSuite) requestReload(c *gc.C, socket string) { - resp, err := newRequest(c, socket, "/reload") + resp, err := newRequest(c, socket, "/reload", http.MethodPost) c.Assert(err, jc.ErrorIsNil) c.Assert(resp.StatusCode, gc.Equals, http.StatusNoContent) } func (s *workerSuite) ensureReloadRequestRefused(c *gc.C, socket string) { - _, err := newRequest(c, socket, "/reload") + _, err := newRequest(c, socket, "/reload", http.MethodPost) c.Assert(err, jc.ErrorIs, syscall.ECONNREFUSED) } func (s *workerSuite) ensureEndpointNotFound(c *gc.C, socket, method string) { - resp, err := newRequest(c, socket, method) + resp, err := newRequest(c, socket, method, http.MethodPost) c.Assert(err, jc.ErrorIsNil) c.Assert(resp.StatusCode, gc.Equals, http.StatusNotFound) } @@ -394,10 +417,10 @@ func ensureDone(c *gc.C, watcher ConfigWatcher) { } } -func newRequest(c *gc.C, socket, method string) (*http.Response, error) { - serverURL := "http://localhost:8080" + method +func newRequest(c *gc.C, socket, path, method string) (*http.Response, error) { + serverURL := "http://localhost:8080" + path req, err := http.NewRequest( - http.MethodPost, + method, serverURL, nil, )