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

Improve simplestreams fetching #16451

Merged
merged 4 commits into from Oct 18, 2023
Merged

Conversation

hpidcock
Copy link
Member

@hpidcock hpidcock commented Oct 18, 2023

This change improves the handling of errors returned from simplestreams datasource fetching. NotFound and Unauthorized are currently the only two recognised errors. Instead of ignoring connection errors, remote internal server errors, which lead to non-deterministic behaviour, this now treats unexpected errors as errors.

Additionally, this change implements small retry logic at the datasource level for unexpected errors to improve resilience to degraded network conditions.

This change also includes a drive-by fix to stop making live requests during TestNoWarn2xFirstRun.

QA steps

This should fail, but can see it retries:
juju boostrap aws --config image-metadata-url=https://not.worky --debug

Try bootstrap lxd, then set image-metadata-url to https://not.worky, add machine, see failed provisioning, then unset image-metadata-url, retry provisioning, should work.

$ juju bootstrap localhost
$ juju model-config image-metadata-url=https://not.worky
$ juju add-machine --series jammy
$ juju status
<machine should be in error>
$ juju model-config --reset image-metadata-url
$ juju retry-provisioning 0
$ sleep <x>
$ juju status
<all happy>

Documentation changes

N/A

Links

Jenkins test failures due to failure to get image metadata
https://jenkins.juju.canonical.com/job/test-charmhub-test-charmhub-download-aws/1828/consoleText
https://jenkins.juju.canonical.com/job/test-constraints-test-constraints-common-google/1242/consoleText
https://jenkins.juju.canonical.com/job/test-spaces_ec2-test-upgrade-charm-with-bind-aws/1500/consoleText
https://jenkins.juju.canonical.com/job/test-unit-test-unit-series-aws/1241/consoleText

wallyworld
wallyworld previously approved these changes Oct 18, 2023
@hpidcock
Copy link
Member Author

/merge

@jujubot jujubot merged commit 1523180 into juju:2.9 Oct 18, 2023
18 of 19 checks passed
@hpidcock hpidcock mentioned this pull request Oct 18, 2023
jujubot added a commit that referenced this pull request Oct 19, 2023
#16457

Forward ports:
- #16426
- #16451
- #16456

Conflicts:
- apiserver/facades/client/modelupgrader/upgrader_test.go
- core/base/supportedseries_linux_test.go
- core/base/supportedseries_test.go
- environs/bootstrap/bootstrap_test.go
- go.mod
- go.sum
- provider/openstack/local_test.go
- upgrades/upgradevalidation/migrate_test.go
- upgrades/upgradevalidation/upgrade_test.go
- upgrades/upgradevalidation/validation_test.go
@hpidcock hpidcock mentioned this pull request Oct 19, 2023
jujubot added a commit that referenced this pull request Oct 19, 2023
@hpidcock hpidcock mentioned this pull request Oct 19, 2023
jujubot added a commit that referenced this pull request Oct 19, 2023
#16461

Forward ports:
- #16426
- #16435
- #16445
- #16452
- #16451
- #16453
- #16455
- #16456
- #16457
- #16460

Conflicts:
- go.mod
- worker/uniter/op_callbacks.go
- worker/uniter/uniter.go
- worker/uniter/util_test.go
@hpidcock hpidcock mentioned this pull request Oct 19, 2023
@ycliuhw ycliuhw mentioned this pull request Oct 19, 2023
jujubot added a commit that referenced this pull request Oct 20, 2023
#16462

Forward ports:
- #16426
- #16435
- #16445
- #16452
- #16451
- #16453
- #16455
- #16456
- #16457
- #16460
- #16461

Conflicts:
- apiserver/facades/agent/provisioner/imagemetadata_test.go
- environs/bootstrap/bootstrap.go
- environs/simplestreams/datasource.go
- environs/simplestreams/datasource_test.go
- go.sum
- worker/dbaccessor/package_test.go
- worker/firewaller/firewaller_test.go
- worker/modelcache/worker_test.go
- worker/uniter/charm/bundles_test.go
- worker/uniter/uniter_test.go
- worker/uniter/util_test.go
- cmd/juju/status/status_internal_test.go
jujubot added a commit that referenced this pull request Oct 23, 2023
#16470

Merge branch `3.3` into `main`:
- #16426
- #16435
- #16445
- #16452
- #16451
- #16453
- #16455
- #16456
- #16457
- #16460
- #16461
- #16454
- #16394
- #16469

```
# Conflicts:
# apiserver/common/secrets/access.go
# apiserver/common/secrets/access_test.go
# apiserver/common/secrets/drain.go
# apiserver/common/secrets/drain_test.go
# apiserver/common/secrets/mocks/commonsecrets_mock.go
# apiserver/common/secrets/watcher.go
# apiserver/common/secrets/watcher_test.go
# apiserver/facades/agent/provisioner/imagemetadata_test.go
# apiserver/facades/agent/secretsdrain/mocks/modelstate.go
# apiserver/facades/agent/secretsdrain/mocks/secretsprovider.go
# apiserver/facades/agent/secretsdrain/package_test.go
# apiserver/facades/agent/secretsdrain/register.go
# apiserver/facades/agent/secretsdrain/state.go
# cmd/jujud/agent/caasoperator/manifolds.go
# environs/bootstrap/bootstrap.go
# environs/simplestreams/datasource.go
# environs/simplestreams/datasource_test.go
# go.sum
# worker/dbaccessor/package_test.go
# worker/firewaller/firewaller_test.go
# worker/modelcache/worker_test.go
# worker/secretsdrainworker/manifold.go
# worker/secretsdrainworker/manifold_test.go
# worker/secretsdrainworker/package_test.go
# worker/uniter/charm/bundles_test.go
# worker/uniter/uniter_test.go
# worker/uniter/util_test.go
```
@manadart
Copy link
Member

For solutions such as air-gapped ones where the default image metadata sources can not be reached, this is a breaking change.

The reason is that we always check them even when custom metadata URLs are used. We might have valid metadata from the configured source, but we return an error now when we can't connect to the official ones.

See: https://bugs.launchpad.net/juju/+bug/2053003

jujubot added a commit that referenced this pull request Feb 20, 2024
#16932

Introduces a way for models to disable resolving image metadata from offical sources when in an air gapped environment.

Since #16451 stopped ignoring network connection errors, since that would lead to non-deterministic behaviour, we now need a way to disable the inbuilt offical image metadata sources for air gapped environments.

## QA steps

- `lxc image list`
- delete all the `juju/` images
- `juju bootstrap lxd --config image-metadata-defaults-disabled=true`
```
Creating Juju controller "localhost-localhost" on lxd/localhost
Looking for packaged Juju agent version 3.3.2 for amd64
No packaged binary found, preparing local Juju agent binary
To configure your system to better support LXD containers, please see: https://documentation.ubuntu.com/lxd/en/latest/explanation/performance_tuning/
Launching controller instance(s) on localhost/localhost...
ERROR failed to bootstrap model: cannot start bootstrap instance in availability zone "harry": no matching image found
```

## Documentation changes

Check `image-metadata-defaults-disabled` and `container-image-metadata-defaults-disabled` model-config documentation.

## Links

**Launchpad bug:** https://bugs.launchpad.net/juju/+bug/2053003

**Jira card:** JUJU-4783
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants