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

Merge develop into 3.0 dqlite #14591

Merged
merged 27 commits into from
Sep 8, 2022

Conversation

manadart
Copy link
Member

@manadart manadart commented Sep 8, 2022

Merge from develop to bring in:

Only trivial mod conflicts.

ycliuhw and others added 27 commits September 2, 2022 17:15
Specifically unit tests for the methods:
- CharmConfig
- GetConfig
- SetMetricCredentials
- ParseSettingsCompatible
- Deploy
…_apiserver_application_tests

juju#14547

Specifically unit tests for the methods:
- CharmConfig
- GetConfig
- SetMetricCredentials
- ParseSettingsCompatible
- Deploy

Some explaination:
- Dropped `TestApplicationDeployWithInvalidStoragePool`: Validating storage constraints is done in AddApplication in the state package (https://github.com/juju/juju/blob/f9939e1df652bdcf4621b24e14179de0530b61ad/state/state.go#L1208), which is outside the scope of this test suite and abstracted away by gomock/faking. As such, the test reduces itself to be identical to `TestApplicationDeployWithStorage`
- Dropped `TestApplicationDeployWithInvalidPlacement` for the same reason.
- Dropped `TestApplicationDeploymentLeavesResourcesOnSuccess` since this is tested implicitly in other tests that succeed and don't include the line leave s.backend.EXPECT().Resources().Return(...)
- Dropped `TestApplicationDeploymentNoTrust` since this tests code outside of this package
- Modify `TestClientApplicationDeployWithBindings` deployParams aren't quite the same due to differences between building bindings for deployApplication and app.EndpointBindings()
- Dropped `TestClientApplicationDeployWithDefaultBindings` since this tests code outside of this package

## Checklist


- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing
- ~[ ] [Integration tests](https://github.com/juju/juju/tree/develop/tests), with comments saying what you're testing~
- ~[ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

Successfully running the unit tests in Jenkins should be enough
juju#14564

We previously removed the steps and `backend` methods for upgrades prior to the 2.9 series, but there were some implementations and their tests left in the code-base.

Here, those are removed. ~1.6kloc, including tests targeting Mongo, which should speed CI up a little too.

## QA steps

All unused methods and associated tests.
- Juju builds.
- State tests pass.

## Documentation changes

None.

## Bug reference

N/A
juju#14571

Add openssh-client to snapcraft for strictly confined `juju ssh` and `juju scp`

## QA steps

Build strict snap, install, test `juju ssh` and `juju scp`.

## Documentation changes

N/A

## Bug reference

N/A
juju#14569

Cleanup secret URI to remove controllerUUID attribute (not used right now).
Also add extra checks when granting access - subject must be alive and disallow CMR grants.

## Checklist

- [X] Code style: imports ordered, good names, simple structure, etc
- [X] Comments saying why design decisions were made
- [X] Go unit tests, with comments saying what you're testing
- ~[ ] [Integration tests](https://github.com/juju/juju/tree/develop/tests), with comments saying what you're testing~
- ~[ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

unit tests
juju#14576

Vault is now an option for storing secret content.

There's 2 model config attributes:
1. `secret-store` which needs to be set to `vault`
2. `secret-store-config` which is json or yaml representing the vault config attributes

The store config is tagged as "secret". It is only available to read/write by model admins.
NB - the config schema supported secret attributes but it did not filter those out for no admin users. Extra code is added to take care of that.

Each model gets its own kv store to hold all secrets for that model. Vault namespaces are support but must be set up outside of juju and passed in via config.
The vault client used by the unit agent is configured with a token used to mediate access to the vault. Secret owners get to update/delete their own secrets. Secret consumers get to read only those secrets shared with them.

Supported vault config attributes include:
- endpoint
- token
- keys (list of unseal keys)
- namespace
- ca-cert
- tls-server-name

The minimum config is endpoint and token.

If unseal keys are supplied, the vault will be unsealed when it is used.
The token needs to be an admin token - no refresh etc is supported at this time.

Because of the need to retain provider ACLs until the secret content is deleted, we also need to tweak the order of removal steps so that the content is removed first.

TODO - model deletion needs to also delete the associated vault kv store.

## Checklist

- [X] Code style: imports ordered, good names, simple structure, etc
- [X] Comments saying why design decisions were made
- [X] Go unit tests, with comments saying what you're testing
- ~[ ] [Integration tests](https://github.com/juju/juju/tree/develop/tests), with comments saying what you're testing~
- ~[ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

```
juju bootstrap lxd test
juju switch controller
test (controller) -> test:admin/controller
 
 juju deploy juju-qa-dummy-source --to 0
Located charm "juju-qa-dummy-source" in charm-hub, revision 6
Deploying "juju-qa-dummy-source" from charm-hub charm "juju-qa-dummy-source", revision 6 in channel stable on jammy
 juju deploy juju-qa-dummy-sink --to 0
Located charm "juju-qa-dummy-sink" in charm-hub, revision 7
Deploying "juju-qa-dummy-sink" from charm-hub charm "juju-qa-dummy-sink", revision 7 in channel stable on jammy
juju relate dummy-sink dummy-source

juju model-config secret-store=vault
juju model-config secret-store-config='{"endpoint":"http://10.64.156.79:8200/","token":"s.2nUMBDHvtIRHNYvRu7KNPcIb"}'
 
juju exec --unit dummy-source/0 secret-add foo=bar
secret:ccb9omnlt5gjn57obg60
juju exec --unit dummy-source/0 "secret-grant -r 0 --unit dummy-sink/0 secret:ccb9omnlt5gjn57obg60"
juju exec --unit dummy-sink/0 "secret-get secret:ccb9omnlt5gjn57obg60"
foo: bar

vault kv list 31830957-157b-491f-8731-ba5740802c0e
Keys
----
ccb9omnlt5gjn57obg60-1
ccbar2nlt5gi21ihd810-1

juju exec --unit dummy-source/0 secret-add foo=bar
secret:ccbar2nlt5gi21ihd810
juju exec --unit dummy-source/0 secret-remove secret:ccbar2nlt5gi21ihd810

vault kv list 31830957-157b-491f-8731-ba5740802c0e
Keys
----
ccb9omnlt5gjn57obg60-1


As an admin user

juju model-config
Attribute From Value
agent-metadata-url default ""
agent-stream default released
...
secrets-store model vault
secret-store-config model '{"endpoint":"http://10.64.156.79:8200/","token":"s.2nUMBDHvtIRHNYvRu7KNPcIb"}'
...

As a non admin user

juju model-config
Attribute From Value
agent-metadata-url default ""
agent-stream default released
...
secrets-store model vault
...
```
juju#14580

When a model is destroyed, ensure any secrets related resources for that model are removed from the secrets store.

Also, for vault, add client-cert and client-key config options. And some improved error handling.

## Checklist

- [X] Code style: imports ordered, good names, simple structure, etc
- [X] Comments saying why design decisions were made
- [X] Go unit tests, with comments saying what you're testing
- ~[ ] [Integration tests](https://github.com/juju/juju/tree/develop/tests), with comments saying what you're testing~
- ~[ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

See juju#14576
Plus, destroy a model with secrets and check vault to ensure there are no remaining policy or secrets left behind.
juju#14237

This PR allows pod spec charms to stay locked to focal for operators allowing the rest of juju to move forward to jammy.

Pod spec charms will eventually be removed, until then, focal should be recent enough.

## QA steps

- Deploy kubeflow-lite, check they use focal charm base image.
- Deploy kubeflow-lite to 2.9, model migrate to 3.0, model upgrade, check they use focal charm base image.
- Deploy kubeflow-lite to 2.9, upgrade to 3.0, check they use focal charm base image.

## Documentation changes

Update pod spec documentation to note that pod spec charms from 3.0 are locked to focal.

## Bug reference

N/A
juju#14590

The address scope matching functionality has long had chicanery in its abstractions to work around Go's lack of generics.

Now that we have them available, we can begin to simplify the logic, also paving the way to introduce common functionality more widely across the `Address` implementations as we need it.

## QA steps

The changes are mechanical only. Passing tests verify correctness.

## Documentation changes

None.

## Bug reference

N/A
@manadart manadart merged commit 1c3d250 into juju:3.0-dqlite Sep 8, 2022
@manadart manadart deleted the develop-into-3.0-dqlite branch September 8, 2022 10:25
jujubot added a commit that referenced this pull request Feb 10, 2023
#15177

The following brings the 3.0-dqlite feature branch into the develop branch.

### Changes

This brings in the dqlite database to sit along side the mongo database. Currently, only leases are implemented in Juju using dqlite, more controller base configuration and data will be subsequently moved over to dqlite once this branch has landed.

#### Leases/Raft

The whole raft implementation has been removed from Juju completely. This includes the following workers:

 - raft backstop
 - raft clusterer
 - raft log
 - raft transport
 - global clock updater

In addition, the raft API implementation has also been removed. Instead, the lease has changed to handle the store (dqlite db) directly, improving readability and complexity.

### Jujud 

The `jujud` agent is now built using musl (specifically musl-gcc). This allows `juju` to be built statically embedding `dqlite` in the same process. There are still some rough edges when building and testing and when this lands, we expect to see some churn to polish any of those issues.

Using `go test` is expected to still work as is, this is a last-minute change so that we can utilize sqlite directly for local tests. If you require to test with dqlite (linux only), then running `-tags="dqlite"` with builds/tests/installs is required. All CI jobs are required to run with the dqlite tag.

Some notes:

 1. `CGO_ENABLED=1` and `CGO_LDFLAGS_ALLOW="(-Wl,-wrap,pthread_create)|(-Wl,-z,now)"` are required if you're using dqlite directly.
 2. You are expected to install musl directly on your system if you want to build, using `make musl-install`. This will require sudo.
 3. For development purposes we will download dqlite `.a` files from an s3 bucket to facilitate the setup process. The tar file is sha256 summed to ensure no MITM. You can build these locally if you want to bypass s3 using `make dqlite-build-lxd`. This will spin up an lxd container to build. **Do not attempt** to run `make dqlite-build` locally, unless you know what you're doing.
 4. To access dqlite from a controller, use `make repl`, this will open up a pseudo repl when you can then explore the database with. `.open <db name>` and then you can use SQL from there.
 5. Cross compilation to other architectures can be done using `GOARCH` and `GOOS` before `make install` or `make build`.

There are probably some things I've forgotten, expect a discourse post soon, which will highlight the development flow.

----

Two conflicts when merging. The resolution was to bring in the secret backends for the manifold tests and the controller config type changed for `DefaultMigrationMinionWaitMax`.

```
CONFLICT (content): Merge conflict in cmd/jujud/agent/machine/manifolds_test.go
CONFLICT (content): Merge conflict in controller/config.go
```

c141b2e (upstream/3.0-dqlite) Merge pull request #15159 from SimonRichardson/system-install-musl-by-default
83656e2 Merge pull request #15156 from SimonRichardson/change-log-ddl
125c19d Fix static-analysis pipeline (#15168)
5abfa24 Merge pull request #15140 from SimonRichardson/allow-testing-on-mac
1dc60f6 (3.0-dqlite) Merge pull request #15153 from SimonRichardson/content-addressable-deps
5a1cd24 Merge pull request #15150 from jack-w-shaw/JUJU-2615_symlink_sudo
4502d63 Merge pull request #15148 from SimonRichardson/better-install-method
88941dd Merge pull request #15134 from SimonRichardson/bootstrap-dqlite-agent-tests
2551ffc Merge pull request #15130 from SimonRichardson/build-jujud-snap
0180a53 (origin/3.0-dqlite, manadart/3.0-dqlite) Merge pull request #15123 from SimonRichardson/fix-manifold-lease-expiry-tests
fdf9cc7 Merge pull request #15115 from SimonRichardson/remove-jujud-main-test-file
bf58843 Merge pull request #15113 from SimonRichardson/remove-api-raftlease-api-client
f9419c0 Merge pull request #15112 from SimonRichardson/fix-initializing-state-twice
334d557 Merge pull request #15108 from SimonRichardson/github-action-go-build
2ee6e1a Merge pull request #15107 from SimonRichardson/cross-building-jujud
5a93305 Merge pull request #15087 from SimonRichardson/ensure-placement-of-file
da95dc0 Merge pull request #15086 from SimonRichardson/more-sudo-changes
7b86376 Merge pull request #15085 from SimonRichardson/sudo-apt-get
c4d4eb6 Merge pull request #15057 from SimonRichardson/dqlite-local-build
0ac79b3 Merge pull request #15061 from manadart/develop-into-3.0-dqlite
adc20f7 Merge pull request #15043 from SimonRichardson/allow-overriding-arch-machine
8c02f22 Merge pull request #15048 from SimonRichardson/static-analysis-fix
4547c06 Merge pull request #15050 from manadart/dqlite-address-option
d51b324 Merge pull request #15049 from manadart/dqlite-bootstrap-options
3801b78 Merge pull request #15047 from manadart/develop-into-3.0-dqlite
22d5247 Merge pull request #15037 from SimonRichardson/standardise-dqlite-build
25640a2 Merge pull request #15036 from SimonRichardson/remove-batch-fsm-controller-config
dfa4cb1 Merge pull request #15041 from manadart/dqlite-fix-mock
caf9481 Merge pull request #15034 from manadart/develop-into-3.0-dqlite
c91985d Merge pull request #15035 from SimonRichardson/remove-typed-lease-error
42d17be Merge pull request #15009 from SimonRichardson/allow-repl-via-juju-ssh
d798238 Merge pull request #15002 from manadart/dqlite-use-lease-store
e4f0d39 Merge pull request #14918 from manadart/3.0-dqlite-lease-store
8315fb7 Merge pull request #14986 from manadart/dqlite-build-from-tags
a73b947 Merge pull request #14927 from manadart/3.0-dqlite-lease-store-interface
1657a1d Merge pull request #14910 from manadart/3.0-dqlite-db-supply
27b23f3 Merge pull request #14909 from manadart/3.0-into-3.0-dqlite
6adff35 Merge pull request #14756 from manadart/develop-into-3.0-dqlite
37d81ff Merge pull request #14717 from manadart/develop-into-3.0-dqlite
fe2edb8 Merge pull request #14671 from manadart/3.0-simplify-dbaccessor
1a09836 Merge pull request #14604 from manadart/3.0-bootstrap-controller-db
5ad011e Merge pull request #14652 from manadart/develop-into-3.0-dqlite
1c3d250 Merge pull request #14591 from manadart/develop-into-3.0-dqlite
229cd3e Merge pull request #14578 from manadart/3.0-dqlite-simplify
9d715ba Merge pull request #14565 from manadart/develop-into-3.0-dqlite
92ffd88 Merge pull request #14466 from manadart/develop-into-3.0-dqlite
57f67ce Merge pull request #14336 from manadart/develop-into-3.0-dqlite
648d354 Merge pull request #14364 from manadart/update-musl
198621d Merge pull request #14241 from manadart/develop-into-3.0-dqlite
0360db6 Merge pull request #14153 from manadart/develop-into-3.0-dqlite
17950b2 Merge pull request #14053 from manadart/develop-into-3.0-dqlite
9452026 Merge pull request #14016 from manadart/develop-into-3.0-dqlite
741baca Merge pull request #13963 from manadart/develop-into-3.0-dqlite
5449603 Merge pull request #13969 from manadart/dqlite-manifolds
7b612a0 Merge pull request #13944 from SimonRichardson/dqlite-develop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants