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
Fetch available agent images with architecture from OCI registry #13459
Conversation
… for CAAS controller;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good start but it seems there's a few things that should be looked at?
return result, errors.Trace(err) | ||
} | ||
if !isModelAdmin && !m.isAdmin { | ||
return result, apiservererrors.ErrPerm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure being an admin is essential just to be able to see what available agent binary versions there are for a model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpuse of this facade method is for upgrade-controller
, it makes sense to make them having the same permission, right? I can remove the isAdmin check here but it will finally fail the upgrade
next step if we do have a version available for uprade.
@@ -121,7 +116,21 @@ func (c *upgradeControllerCommand) getControllerAPI() (ControllerAPI, error) { | |||
return c.NewControllerAPIClient() | |||
} | |||
|
|||
func (c *upgradeControllerCommand) getModelManagerAPI() (ModelManagerAPI, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already on upgradeJujuCommand
- that just needs to be moved to baseUpgradeCommand instead of introducing a new implementation here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can move that from upgradeJujuCommand to baseUpgradeCommand unless we want to move the modelcmd.ModelCommandBase from upgradeJujuCommand to the upgradeJujuCommand as well.
The
upgrade-controller
command currently fetches available agent versions for upgrading on the client-side which makes the command itself complicated and difficult to manage consistencies. Therefore, we moved the agent versions (jujud-operator
image tags) fetching for CAAS to the server-side and theupgrade-controller
command calls theToolVersions
facade method instead (This also causes the modelmanager facade upgraded to version10
). Fetching simple streams for IAAS is still aTODO
and we should move it to the server-side as well in the future.To construct a
core.Tools
for theToolVersions
facade to return, we need the architecture of the agent. So we also implemented two new registry APIs(manifests
andblobs
) forArch
fetching for OCI images (Because of this, we also migrated some of the registry providers from APIv1
tov2
for public repositories.).Checklist
Requires a pylibjuju changeAdded integration tests for the PRQA steps
https://discourse.charmhub.io/t/advanced-config-to-host-your-own-juju-operator-images/5079
Documentation changes
https://discourse.charmhub.io/t/advanced-config-to-host-your-own-juju-operator-images/5079
Bug reference
https://bugs.launchpad.net/juju/+bug/1944801