Skip to content

Commit

Permalink
Merge pull request juju#14617 from hmlanigan/state-converter-manifold
Browse files Browse the repository at this point in the history 
juju#14617

While looking at a migration bug, I found a manifold called "unconverted-api-workers" started in the machine agent. These are workers never converted when the manifold was introduced way back in time.

Convert the state-converter worker to have it's own manifold, one step closer to removing the unconverted-api-workers. This also necessitated a new manifold flag to run the worker on machine which are not controllers.
## QA steps

```sh
juju bootstrap localhost test-enable-ha
juju deploy ubuntu
juju add-machine -m controller -n 2

# once the ubuntu unit is up and running, login and verify the state-converter worker is running by reviewing juju_engine_report output.

juju switch controller

# once machine-2 in the controller model is up and running, ssh in and verify the state-converter worker is running by reviewing juju_engine_report output.

juju enable-ha --to 1,2

# watch show controller output for ha to be fully enabled.
juju show-controller

# ssh to machine-2 and verify the state-converter worker is NOT running by reviewing juju_engine_report output.
# verify that the juju-db snap has been installed.
# verify that the juju agent was restarted the machine-2.log

```
  • Loading branch information
@jujubot
jujubot authored Sep 16, 2022
2 parents a6f7552 + 22cc4e0 commit 6ac8f81
Show file tree
Hide file tree
Showing 16 changed files with 1,105 additions and 230 deletions.
3 changes: 3 additions & 0 deletions cmd/jujud/agent/engine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ var (
"deployer",
"disk-manager",
"fan-configurer",
"is-controller-flag",
"is-not-controller-flag",
// "host-key-reporter", not stable, exits when done
"log-sender",
"logging-config-updater",
Expand All @@ -126,6 +128,7 @@ var (
"proxy-config-updater",
"reboot-executor",
"ssh-authkeys-updater",
"state-converter",
"storage-provisioner",
"upgrade-series",
"unconverted-api-workers",
Expand Down
14 changes: 0 additions & 14 deletions cmd/jujud/agent/machine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ import (
"github.com/juju/juju/upgrades"
jworker "github.com/juju/juju/worker"
workercommon "github.com/juju/juju/worker/common"
"github.com/juju/juju/worker/conv2state"
"github.com/juju/juju/worker/deployer"
"github.com/juju/juju/worker/gate"
"github.com/juju/juju/worker/introspection"
Expand Down Expand Up @@ -791,19 +790,6 @@ func (a *MachineAgent) startAPIWorkers(apiConn api.Connection) (_ worker.Worker,
if result.Code != 0 {
return nil, errors.New(fmt.Sprintf("cannot patch /etc/update-manager/release-upgrades: %s", result.Stderr))
}
} else {
_ = runner.StartWorker("stateconverter", func() (worker.Worker, error) {
// TODO(fwereade): this worker needs its own facade.
facade := apimachiner.NewState(apiConn)
handler := conv2state.New(facade, a)
w, err := watcher.NewNotifyWorker(watcher.NotifyConfig{
Handler: handler,
})
if err != nil {
return nil, errors.Annotate(err, "cannot start controller promoter worker")
}
return w, nil
})
}
return runner, nil
}
Expand Down
19 changes: 17 additions & 2 deletions cmd/jujud/agent/machine/manifolds.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ import (
"github.com/juju/juju/worker/singular"
workerstate "github.com/juju/juju/worker/state"
"github.com/juju/juju/worker/stateconfigwatcher"
"github.com/juju/juju/worker/stateconverter"
"github.com/juju/juju/worker/storageprovisioner"
"github.com/juju/juju/worker/syslogger"
"github.com/juju/juju/worker/terminationworker"
Expand Down Expand Up @@ -344,7 +345,7 @@ func commonManifolds(config ManifoldsConfig) dependency.Manifolds {

// Each machine agent has a flag manifold/worker which
// reports whether or not the agent is a controller.
isControllerFlagName: isControllerFlagManifold(),
isControllerFlagName: isControllerFlagManifold(true),

// The stateconfigwatcher manifold watches the machine agent's
// configuration and reports if state serving info is
Expand Down Expand Up @@ -1014,11 +1015,17 @@ func IAASManifolds(config ManifoldsConfig) dependency.Manifolds {
NewClient: instancemutater.NewClient,
NewWorker: instancemutater.NewContainerWorker,
})),

syslogName: syslogger.Manifold(syslogger.ManifoldConfig{
NewWorker: syslogger.NewWorker,
NewLogger: syslogger.NewSyslog,
}),
// isNotControllerFlagName is only used for the stateconverter,
isNotControllerFlagName: isControllerFlagManifold(false),
stateConverterName: ifNotController(ifNotMigrating(stateconverter.Manifold(stateconverter.ManifoldConfig{
AgentName: agentName,
APICallerName: apiCallerName,
Logger: loggo.GetLogger("juju.worker.stateconverter"),
}))),
}

return mergeManifolds(config, manifolds)
Expand Down Expand Up @@ -1096,6 +1103,12 @@ var ifController = engine.Housing{
},
}.Decorate

var ifNotController = engine.Housing{
Flags: []string{
isNotControllerFlagName,
},
}.Decorate

var ifRaftLeader = engine.Housing{
Flags: []string{
raftFlagName,
Expand Down Expand Up @@ -1170,6 +1183,7 @@ const (
leaseClockUpdaterName = "lease-clock-updater"
isPrimaryControllerFlagName = "is-primary-controller-flag"
isControllerFlagName = "is-controller-flag"
isNotControllerFlagName = "is-not-controller-flag"
instanceMutaterName = "instance-mutater"
txnPrunerName = "transaction-pruner"
certificateWatcherName = "certificate-watcher"
Expand All @@ -1183,6 +1197,7 @@ const (
certificateUpdaterName = "certificate-updater"
auditConfigUpdaterName = "audit-config-updater"
leaseManagerName = "lease-manager"
stateConverterName = "state-converter"

upgradeSeriesWorkerName = "upgrade-series"

Expand Down
19 changes: 19 additions & 0 deletions cmd/jujud/agent/machine/manifolds_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func (s *ManifoldsSuite) TestManifoldNamesIAAS(c *gc.C) {
"http-server-args",
"instance-mutater",
"is-controller-flag",
"is-not-controller-flag",
"is-primary-controller-flag",
"lease-clock-updater",
"lease-manager",
Expand Down Expand Up @@ -112,6 +113,7 @@ func (s *ManifoldsSuite) TestManifoldNamesIAAS(c *gc.C) {
"ssh-identity-writer",
"state",
"state-config-watcher",
"state-converter",
"storage-provisioner",
"syslog",
"termination-signal-handler",
Expand Down Expand Up @@ -237,6 +239,7 @@ func (s *ManifoldsSuite) TestMigrationGuardsUsed(c *gc.C) {
"http-server",
"http-server-args",
"is-controller-flag",
"is-not-controller-flag",
"is-primary-controller-flag",
"lease-clock-updater",
"lease-manager",
Expand Down Expand Up @@ -647,6 +650,8 @@ var expectedMachineManifoldsWithDependenciesIAAS = map[string][]string{

"is-controller-flag": {"agent", "state-config-watcher"},

"is-not-controller-flag": {"agent", "state-config-watcher"},

"is-primary-controller-flag": {
"agent",
"api-caller",
Expand Down Expand Up @@ -994,6 +999,20 @@ var expectedMachineManifoldsWithDependenciesIAAS = map[string][]string{

"state-config-watcher": {"agent"},

"state-converter": {
"agent",
"api-caller",
"api-config-watcher",
"is-not-controller-flag",
"migration-fortress",
"migration-inactive-flag",
"state-config-watcher",
"upgrade-check-flag",
"upgrade-check-gate",
"upgrade-steps-flag",
"upgrade-steps-gate",
},

"storage-provisioner": {
"agent",
"api-caller",
Expand Down
16 changes: 7 additions & 9 deletions cmd/jujud/agent/machine/stateflag.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,19 @@
package machine

import (
"github.com/juju/errors"
"github.com/juju/worker/v3"
"github.com/juju/worker/v3/dependency"

"github.com/juju/juju/cmd/jujud/agent/engine"
)

// isControllerFlagManifold returns a dependency.Manifold that requires state
// config.
// isControllerFlagManifold returns a dependency.Manifold that indicates
// the state config is present or not depending on the arg.
// It returns a worker implementing engine.Flag, whose Check method returns
// whether state config is present on the machine.
func isControllerFlagManifold() dependency.Manifold {
// True in 2 cases:
// 1) state config is present on the machine and arg is True
// 2) state config is not present on the machine and arg is False.
func isControllerFlagManifold(yes bool) dependency.Manifold {
return dependency.Manifold{
Inputs: []string{stateConfigWatcherName},
Output: engine.FlagOutput,
Expand All @@ -24,10 +25,7 @@ func isControllerFlagManifold() dependency.Manifold {
if err := context.Get(stateConfigWatcherName, &haveStateConfig); err != nil {
return nil, err
}
if !haveStateConfig {
return nil, errors.Annotate(dependency.ErrMissing, "no state config detected")
}
return engine.NewStaticFlagWorker(true), nil
return engine.NewStaticFlagWorker(haveStateConfig && yes || !haveStateConfig && !yes), nil
},
}
}
121 changes: 0 additions & 121 deletions worker/conv2state/converter_test.go
View file

This file was deleted.

60 changes: 0 additions & 60 deletions worker/conv2state/fakes_test.go
View file

This file was deleted.

Loading

0 comments on commit 6ac8f81

Please sign in to comment.