Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[JUJU-1816] give the state converter worker it's own manifold #14617

Merged
merged 11 commits into from
Sep 16, 2022
Merged

[JUJU-1816] give the state converter worker it's own manifold #14617

merged 11 commits into from
Sep 16, 2022

Conversation

hmlanigan
Copy link
Member

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

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

@hmlanigan
Add manifold for conv2state worker.
40b241e
@hmlanigan
Convert conv2state worker to use a manifold of it's own.
b3d202e 
Pull out from under the unconverted-api-workers.

Add IfNotController flag for machine manifolds. The conv2state worker
converts a regular juju machine to a controller, doesn't need to run on
a machine already converted.
@hmlanigan
rename conv2state package to stateconverter
6335f46
@hmlanigan
Fix the business logic
399efdf 
Return true if this is a controller and we want a controller, or
this isn't a controller and we do not want a controller.
@hmlanigan
Convert unit tests to gomock and enable testing of new Manifold.
0500c3a
@hmlanigan
Add interface headers
43b43fc
@hmlanigan
isNotController should be IAAS, not common,
8a5c8f1 
Update tests for the stateconverter worker
@hmlanigan hmlanigan added the 2.9 label Sep 15, 2022
@hmlanigan hmlanigan changed the title give the state converter worker it's own manifold [JUJU-1816] give the state converter worker it's own manifold Sep 15, 2022
wallyworld
wallyworld approved these changes Sep 15, 2022
View reviewed changes
Copy link
Member

@wallyworld wallyworld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this

cmd/jujud/agent/machine/stateflag.go Outdated Show resolved Hide resolved
worker/stateconverter/converter_test.go Outdated Show resolved Hide resolved
worker/stateconverter/interfaces.go Outdated Show resolved Hide resolved
@hmlanigan
Copy link
Member Author

/merge

@jujubot jujubot merged commit 6ac8f81 into juju:2.9 Sep 16, 2022
@hmlanigan hmlanigan deleted the state-converter-manifold branch September 16, 2022 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wallyworld wallyworld wallyworld approved these changes

Assignees

@wallyworld wallyworld

@basebandit basebandit

Labels
2.9
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hmlanigan @wallyworld @jujubot @basebandit