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

Implement elasticContainerRegistry; #13350

Merged
merged 13 commits into from
Oct 1, 2021
Merged

Implement elasticContainerRegistry; #13350

merged 13 commits into from
Oct 1, 2021

Conversation

ycliuhw
Copy link
Member

@ycliuhw ycliuhw commented Sep 22, 2021 •
edited
Loading

This PR introduces AWS ecr support for

  • pulling jujud-operator, juju-db and charm-base images from k8s controllers, application operators, and sidecar pods;
  • fetching available controller versions to upgrade for upgrade-controller command;

Tested registries are(the rest of the registries will be implemented and tested in the following PRs):

Registry Public Private
azurecr.io
docker.io ✅ ✅
ecr ❌ ✅
gcr.io ✅ ✅
ghcr.io ❌ ✅
registry.gitlab.com ❌ ✅
quay.io ✅ ✅

Checklist

  • Requires a pylibjuju change
  • Added integration tests for the PR
  • Added or updated doc.go related to packages changed
  • Comments answer the question of why design decisions were made

QA steps

$ cat ecr.json
{
    "serveraddress": "xxxx.dkr.ecr.eu-west-1.amazonaws.com",
    "repository": "xxxx.dkr.ecr.eu-west-1.amazonaws.com",
    "username": "<aws_access_key_id>",
    "password": "<aws_secret_access_key>",
    "region": "ap-southeast-2"
}

$ juju bootstrap microk8s k1 --config caas-image-repo="'$(cat ecr.json)'" 

$ mkubectl -ncontroller-k1 get secret/juju-image-pull-secret -o json | jq -r '.data[".dockerconfigjson"]' | base64 --decode | jq
{
  "auths": {
    "xxxx.dkr.ecr.eu-west-1.amazonaws.com": {
      "auth": "xxxx==",
      "username": "<aws_access_key_id>",
      "password": "<aws_secret_access_key>",
      "region": "ap-southeast-2"
      "serveraddress": "xxxx.dkr.ecr.eu-west-1.amazonaws.com",
    }
  }
}

$ mkubectl -ncontroller-k1 get pod -o json | jq '.items[].spec.containers[].image'
"xxxx.dkr.ecr.eu-west-1.amazonaws.com/jujud-operator:2.9.15"
"xxxx.dkr.ecr.eu-west-1.amazonaws.com/juju-db:4.0"
"xxxx.dkr.ecr.eu-west-1.amazonaws.com/jujud-operator:2.9.15"

$ juju upgrade-controller --agent-stream=develop

Documentation changes

https://discourse.charmhub.io/t/initial-private-registry-support/5079

Bug reference

https://bugs.launchpad.net/juju/+bug/1935830
https://bugs.launchpad.net/juju/+bug/1940820
https://bugs.launchpad.net/juju/+bug/1935953

@ycliuhw
Copy link
Member Author

ycliuhw commented Sep 22, 2021

integrate Azure and AWS Container Registry

@ycliuhw
Copy link
Member Author

ycliuhw commented Sep 23, 2021

integrate AWS Container Registry

@ycliuhw ycliuhw marked this pull request as ready for review September 24, 2021 09:33
@hpidcock hpidcock added the 2.9 label Sep 27, 2021
@ycliuhw ycliuhw changed the title Draft elasticContainerRegistry; Implement elasticContainerRegistry; Sep 28, 2021
wallyworld
wallyworld approved these changes Sep 28, 2021
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.

Looks good but a few questions before landing, especially the ExpiresAt config attributes and where they should be placed, and the short refresh time

docker/auth.go Outdated Show resolved Hide resolved
docker/registry/internal/acr.go Outdated Show resolved Hide resolved
docker/registry/internal/ecr.go Outdated Show resolved Hide resolved
docker/registry/internal/ecr.go Outdated Show resolved Hide resolved
worker/caasmodelconfigmanager/worker.go Outdated Show resolved Hide resolved
docker/registry/internal/ecr.go Outdated Show resolved Hide resolved
@ycliuhw ycliuhw force-pushed the feature/ecr branch 2 times, most recently from a3482e0 to 861e261 Compare September 30, 2021 09:59
wallyworld
wallyworld approved these changes Sep 30, 2021
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.

The token refresh logic needs fixing to avoid a memory leak

docker/registry/internal/gcr.go Outdated Show resolved Hide resolved
worker/caasmodelconfigmanager/worker.go Outdated Show resolved Hide resolved
docker/auth.go Outdated Show resolved Hide resolved
@ycliuhw
Copy link
Member Author

ycliuhw commented Oct 1, 2021

$$merge$$

@jujubot jujubot merged commit d528867 into juju:2.9 Oct 1, 2021
@wallyworld wallyworld mentioned this pull request Oct 5, 2021
Merged
jujubot added a commit that referenced this pull request Oct 5, 2021
@jujubot
Merge pull request #13385 from wallyworld/merge-2.9-20211005
0482afd 
#13385

Merge 2.9

#13363 Use a mock for upgradedatabase test clock
#13347 Update oracle api and fix bootstrap
#13343 Bootstrap test refactor
#13372 Pass HTTP Client through
#13364 Make upgrade smoke test workflow more robust
#13374 For older machine agents, there needs to be a symlink for the series based tools
#13371 Stick the secret revision in the URL path not as a query parameter
#13375 CLI: NO_COLOR support
#13365 Pin kubeflow test to a fixed sha;
#13376 Rename secret status pending to staged
#13380 Add secret grant/revoke hook command CLI
#13350 Implement elasticContainerRegistry;
#13378 Metrics: Reduce pubsub cardinality
#13377 State: Logs already exist
#13381 Update Pebble to add replan, wait-change, and one-shot commands
#13383 LXD network retrieval efficiency

```
# Conflicts:
# caas/kubernetes/provider/bootstrap_test.go
# go.mod
# go.sum
#
```
## QA steps

See PRs
@ycliuhw
Copy link
Member Author

ycliuhw commented Oct 6, 2021

integrate more registry support, for example gcr, azure, etc.

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
No one assigned
Labels
2.9
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ycliuhw @wallyworld @hpidcock @jujubot