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.

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

refactor: remove state policy #18260

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SimonRichardson
Copy link
Member

Removes the state policy package altogether. It's gone!

We need to be more aggressive with our cuts. We have an issue where state code depends on domain services, and for model migration, domain services depend on state code. We're essentially stuck, so we need to solve this impasse. The solution provided here is just to remove the domain services from the state. The domain services shouldn't have been added to state, but it did allow us to keep moving at the time.

It should be "policy" not to inject services into the state package, as we end up locking up at some point. Instead, we should either implement sane dependencies and fix them when implementing the epic, or lift the dependency out of state and push it down as an argument to method on state.

For policy, I've used the former and in doing so, I've ripped out all the policy.

QA steps

We need to bootstrap and test on both machine and containers. Then we need to be able to migrate from 3.6 to main.

$ snap install juju --channel 3.6/edge
$ /snap/bin/juju bootstrap lxd src
$ /snap/bin/juju add-model moveme
$ /snap/bin/juju deploy ubuntu

Use this branch

$ juju bootstrap lxd dst
$ juju migrate src:moveme dst

Repeat this process for microk8s.

@Aflynn50
Copy link
Contributor

Tested this out with #18206

More issues of state code depending on domain 😃😃 @SimonRichardson @manadart

runtime/debug.Stack()
        /snap/go/10730/src/runtime/debug/stack.go:26 +0x5e
github.com/juju/juju/domain.(*StateBase).DB(0xc002f09500)
        /home/aflynn/Canonical/Juju/juju3/domain/state.go:71 +0x1eb
github.com/juju/juju/domain/storage/state.StoragePoolState.GetStoragePoolByName({0xc002df8640?}, {0x7fcf050, 0xbf45140}, {0xc002cd9ef0, 0xa})
        /home/aflynn/Canonical/Juju/juju3/domain/storage/state/storagepool.go:357 +0x4e
github.com/juju/juju/domain/storage/service.(*StoragePoolService).GetStoragePoolByName(0xc003112c00, {0x7fcf050, 0xbf45140}, {0xc002cd9ef0, 0xa})
        /home/aflynn/Canonical/Juju/juju3/domain/storage/service/storagepool.go:225 +0x271
github.com/juju/juju/state.poolStorageProvider(0x7f36c38?, {0xc002cd9ef0, 0xa})
        /home/aflynn/Canonical/Juju/juju3/state/storage.go:1901 +0x69
github.com/juju/juju/state.isDetachableVolumePool(0xc003118a30?, {0xc002cd9ef0?, 0x0?})
        /home/aflynn/Canonical/Juju/juju3/state/volume.go:699 +0x1c
github.com/juju/juju/state.(*importer).addVolume(0xc002db99a0, {0x7ffb9a0, 0xc001f25880}, 0xc002930000)
        /home/aflynn/Canonical/Juju/juju3/state/migration_import.go:2029 +0x3fc
github.com/juju/juju/state.(*importer).volumes(0xc002db99a0)
        /home/aflynn/Canonical/Juju/juju3/state/migration_import.go:1989 +0xd6
github.com/juju/juju/state.(*importer).storage(0xc002db99a0)
        /home/aflynn/Canonical/Juju/juju3/state/migration_import.go:1846 +0x2f
github.com/juju/juju/state.(*Controller).Import(0xc002b736f0, {0x8029278, 0xc002d68008}, 0xc002dca900, 0xc0025c7e90)
        /home/aflynn/Canonical/Juju/juju3/state/migration_import.go:178 +0xbf6
github.com/juju/juju/internal/migration.(*ModelImporter).ImportModel(0xc002b7a0e0, {0x7fcf368, 0xc002a46210}, {0xc002ca6000?, 0x10?, 0x6be6120?})
        /home/aflynn/Canonical/Juju/juju3/internal/migration/migration.go:220 +0x147
github.com/juju/juju/apiserver/facades/controller/migrationtarget.(*API).Import(0x1?, {0x7fcf368?, 0xc002a46210?}, {{0xc002ca6000, 0xa39a, 0xa39b}, {0x0, 0x0, 0x0}, {0x0, ...}, ...})
        /home/aflynn/Canonical/Juju/juju3/apiserver/facades/controller/migrationtarget/migrationtarget.go:253 +0x54
reflect.Value.call({0x6fb72a0?, 0xc0025b1180?, 0x9c49e9?}, {0x74e2f8a, 0x4}, {0xc002ace1b0, 0x2, 0x6be6120?})
        /snap/go/10730/src/reflect/value.go:581 +0xca6
reflect.Value.Call({0x6fb72a0?, 0xc0025b1180?, 0x6e2bae0?}, {0xc002ace1b0?, 0xc002c01f80?, 0x10?})
        /snap/go/10730/src/reflect/value.go:365 +0xb9
github.com/juju/juju/internal/rpcreflect.newMethod.func6({0x7fcf368, 0xc002a46210}, {0x6fb72a0?, 0xc0025b1180?, 0xc002a462a0?}, {0x6e2bae0?, 0xc0018d99e0?, 0xc0023bf9e0?})
        /home/aflynn/Canonical/Juju/juju3/internal/rpcreflect/type.go:348 +0xc8
github.com/juju/juju/apiserver.(*srvCaller).Call(0xc000f77c50, {0x7fcf368, 0xc002a46210}, {0x0?, 0x738b09d98cb8?}, {0x6e2bae0?, 0xc0018d99e0?, 0x738b0b41a0d0?})
        /home/aflynn/Canonical/Juju/juju3/apiserver/root.go:428 +0xa3
github.com/juju/juju/rpc.(*Conn).callRequest(0xc0025b0d20, {0x7fcf368, 0xc002a46210}, {{0x7fca560, 0xc000f77c50}, 0x78057b0, {0x2, {{0xc001ac5530, 0xf}, 0x3, ...}, ...}}, ...)
        /home/aflynn/Canonical/Juju/juju3/rpc/server.go:651 +0xb3
github.com/juju/juju/rpc.(*Conn).runRequest.func2()
        /home/aflynn/Canonical/Juju/juju3/rpc/server.go:625 +0x5f
github.com/juju/juju/rpc.(*Conn).withTrace.func1({0x7fcf368?, 0xc002a462a0?})
        /home/aflynn/Canonical/Juju/juju3/rpc/server.go:640 +0x13
runtime/pprof.Do({0x7fcf368?, 0xc002a46240?}, {{0xc0025a6260?, 0x10?, 0xc001e5ac88?}}, 0xc0023bfcf8)
        /snap/go/10730/src/runtime/pprof/runtime.go:51 +0x8c
github.com/juju/juju/rpc.(*Conn).withTrace(0x10?, {0x7fcf368?, 0xc002a46210?}, {{0xc001ac5530, 0xf}, 0x3, {0x0, 0x0}, {0xc001ac5518, 0x6}}, ...)
        /home/aflynn/Canonical/Juju/juju3/rpc/server.go:639 +0x3c5
github.com/juju/juju/rpc.(*Conn).runRequest(0xc0025b0d20, {{0x7fca560, 0xc000f77c50}, 0x78057b0, {0x2, {{0xc001ac5530, 0xf}, 0x3, {0x0, 0x0}, ...}, ...}}, ...)
        /home/aflynn/Canonical/Juju/juju3/rpc/server.go:624 +0x285
created by github.com/juju/juju/rpc.(*Conn).handleRequest in goroutine 4850
        /home/aflynn/Canonical/Juju/juju3/rpc/server.go:520 +0x5fd

The state policy is now gone. We don't need this in the future, it
was a bad design and overly complex. Policy is a terrible word here.

Anyway, this is gone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants