Skip to content

Commit

Permalink
fix(upgrade): fix broken upgrade on k8s
Browse files Browse the repository at this point in the history 
Upgrades on k8s were broken duue to the oci image fat manifest format not being supported by Juju.
  • Loading branch information
@wallyworld
wallyworld committed Jul 18, 2024
1 parent b6ba452 commit f4d5b45
Show file tree
Hide file tree
Showing 15 changed files with 221 additions and 109 deletions.
12 changes: 9 additions & 3 deletions apiserver/facades/client/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/juju/juju/apiserver/facade"
"github.com/juju/juju/cloudconfig/podcfg"
"github.com/juju/juju/controller"
"github.com/juju/juju/core/arch"
"github.com/juju/juju/core/cache"
"github.com/juju/juju/core/leadership"
"github.com/juju/juju/core/multiwatcher"
Expand Down Expand Up @@ -327,6 +328,11 @@ func (c *Client) toolVersionsForCAAS(args params.FindToolsParams, streamsVersion
if err != nil {
return result, errors.Trace(err)
}

wantArch := args.Arch
if wantArch == "" {
wantArch = arch.DefaultArchitecture
}
for _, tag := range tags {
number := tag.AgentVersion()
if number.Compare(current) <= 0 {
Expand All @@ -351,21 +357,21 @@ func (c *Client) toolVersionsForCAAS(args params.FindToolsParams, streamsVersion
continue
}
}
arch, err := reg.GetArchitecture(imageName, number.String())
arches, err := reg.GetArchitectures(imageName, number.String())
if errors.IsNotFound(err) {
continue
}
if err != nil {
return result, errors.Annotatef(err, "cannot get architecture for %s:%s", imageName, number.String())
}
if args.Arch != "" && arch != args.Arch {
if !set.NewStrings(arches...).Contains(wantArch) {
continue
}
tools := tools.Tools{
Version: version.Binary{
Number: number,
Release: coreos.HostOSTypeName(),
Arch: arch,
Arch: wantArch,
},
}
result.List = append(result.List, &tools)
Expand Down
31 changes: 21 additions & 10 deletions apiserver/facades/client/client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,15 @@ func (s *findToolsSuite) getModelConfig(c *gc.C, agentVersion string) *config.Co
return mCfg
}

func (s *findToolsSuite) TestFindToolsCAASReleasedDefault(c *gc.C) {
s.assertFindToolsCAASReleased(c, "", "amd64")
}

func (s *findToolsSuite) TestFindToolsCAASReleased(c *gc.C) {
s.assertFindToolsCAASReleased(c, "arm64", "arm64")
}

func (s *findToolsSuite) assertFindToolsCAASReleased(c *gc.C, wantArch, expectArch string) {
ctrl := gomock.NewController(c)
defer ctrl.Finish()

Expand All @@ -502,6 +510,9 @@ func (s *findToolsSuite) TestFindToolsCAASReleased(c *gc.C) {
{Version: version.MustParseBinary("2.9.9-ubuntu-amd64")},
{Version: version.MustParseBinary("2.9.10-ubuntu-amd64")},
{Version: version.MustParseBinary("2.9.11-ubuntu-amd64")},
{Version: version.MustParseBinary("2.9.9-ubuntu-arm64")},
{Version: version.MustParseBinary("2.9.10-ubuntu-arm64")},
{Version: version.MustParseBinary("2.9.11-ubuntu-arm64")},
}
s.PatchValue(&coreos.HostOS, func() coreos.OSType { return coreos.Ubuntu })

Expand All @@ -513,7 +524,7 @@ func (s *findToolsSuite) TestFindToolsCAASReleased(c *gc.C) {
authorizer.EXPECT().HasPermission(permission.WriteAccess, coretesting.ModelTag).Return(nil),

backend.EXPECT().Model().Return(model, nil),
toolsFinder.EXPECT().FindAgents(common.FindAgentsParams{MajorVersion: 2}).
toolsFinder.EXPECT().FindAgents(common.FindAgentsParams{MajorVersion: 2, Arch: wantArch}).
Return(simpleStreams, nil),
model.EXPECT().Type().Return(state.ModelTypeCAAS),
model.EXPECT().Config().Return(s.getModelConfig(c, "2.9.9"), nil),
Expand All @@ -536,8 +547,8 @@ func (s *findToolsSuite) TestFindToolsCAASReleased(c *gc.C) {
image.NewImageInfo(version.MustParse("2.9.11")),
image.NewImageInfo(version.MustParse("2.9.12")), // skip: it's not released in simplestream yet.
}, nil),
registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.10").Return("amd64", nil),
registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.11").Return("amd64", nil),
registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.10").Return([]string{"amd64", "arm64"}, nil),
registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.11").Return([]string{"amd64", "arm64"}, nil),
registryProvider.EXPECT().Close().Return(nil),
)

Expand All @@ -558,12 +569,12 @@ func (s *findToolsSuite) TestFindToolsCAASReleased(c *gc.C) {
},
)
c.Assert(err, jc.ErrorIsNil)
result, err := api.FindTools(params.FindToolsParams{MajorVersion: 2})
result, err := api.FindTools(params.FindToolsParams{MajorVersion: 2, Arch: wantArch})
c.Assert(err, jc.ErrorIsNil)
c.Assert(result, gc.DeepEquals, params.FindToolsResult{
List: []*tools.Tools{
{Version: version.MustParseBinary("2.9.10-ubuntu-amd64")},
{Version: version.MustParseBinary("2.9.11-ubuntu-amd64")},
{Version: version.MustParseBinary("2.9.10-ubuntu-" + expectArch)},
{Version: version.MustParseBinary("2.9.11-ubuntu-" + expectArch)},
},
})
}
Expand Down Expand Up @@ -618,10 +629,10 @@ func (s *findToolsSuite) TestFindToolsCAASNonReleased(c *gc.C) {
image.NewImageInfo(version.MustParse("2.9.12")),
image.NewImageInfo(version.MustParse("2.9.13")), // skip: it's not released in simplestream yet.
}, nil),
registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.10.1").Return("amd64", nil),
registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.10").Return("amd64", nil),
registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.11").Return("amd64", nil),
registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.12").Return("", errors.NotFoundf("2.9.12")), // This can only happen on a non-official registry account.
registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.10.1").Return([]string{"amd64", "arm64"}, nil),
registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.10").Return([]string{"amd64", "arm64"}, nil),
registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.11").Return([]string{"amd64", "arm64"}, nil),
registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.12").Return(nil, errors.NotFoundf("2.9.12")), // This can only happen on a non-official registry account.
registryProvider.EXPECT().Close().Return(nil),
)

Expand Down
12 changes: 9 additions & 3 deletions apiserver/facades/client/modelupgrader/findagents.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/juju/juju/apiserver/common"
"github.com/juju/juju/cloudconfig/podcfg"
"github.com/juju/juju/controller"
"github.com/juju/juju/core/arch"
coreos "github.com/juju/juju/core/os"
"github.com/juju/juju/docker"
envtools "github.com/juju/juju/environs/tools"
Expand Down Expand Up @@ -121,6 +122,11 @@ func (m *ModelUpgraderAPI) agentVersionsForCAAS(
if err != nil {
return nil, errors.Trace(err)
}

wantArch := args.Arch
if wantArch == "" {
wantArch = arch.DefaultArchitecture
}
for _, tag := range tags {
number := tag.AgentVersion()
if args.MajorVersion > 0 {
Expand All @@ -145,21 +151,21 @@ func (m *ModelUpgraderAPI) agentVersionsForCAAS(
continue
}
}
arch, err := reg.GetArchitecture(imageName, number.String())
arches, err := reg.GetArchitectures(imageName, number.String())
if errors.Is(err, errors.NotFound) {
continue
}
if err != nil {
return nil, errors.Annotatef(err, "cannot get architecture for %s:%s", imageName, number.String())
}
if args.Arch != "" && arch != args.Arch {
if !set.NewStrings(arches...).Contains(wantArch) {
continue
}
tools := coretools.Tools{
Version: version.Binary{
Number: number,
Release: coreos.HostOSTypeName(),
Arch: arch,
Arch: wantArch,
},
}
result = append(result, &tools)
Expand Down
43 changes: 28 additions & 15 deletions apiserver/facades/client/modelupgrader/upgrader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,15 @@ func (s *modelUpgradeSuite) TestFindToolsIAAS(c *gc.C) {
})
}

func (s *modelUpgradeSuite) TestFindToolsCAASReleasedDefault(c *gc.C) {
s.assertFindToolsCAASReleased(c, "", "amd64")
}

func (s *modelUpgradeSuite) TestFindToolsCAASReleased(c *gc.C) {
s.assertFindToolsCAASReleased(c, "arm64", "arm64")
}

func (s *modelUpgradeSuite) assertFindToolsCAASReleased(c *gc.C, wantArch, expectArch string) {
ctrl, api := s.getModelUpgraderAPI(c)
defer ctrl.Finish()

Expand All @@ -786,13 +794,17 @@ func (s *modelUpgradeSuite) TestFindToolsCAASReleased(c *gc.C) {
{Version: version.MustParseBinary("2.9.9-ubuntu-amd64")},
{Version: version.MustParseBinary("2.9.10-ubuntu-amd64")},
{Version: version.MustParseBinary("2.9.11-ubuntu-amd64")},
{Version: version.MustParseBinary("2.9.9-ubuntu-arm64")},
{Version: version.MustParseBinary("2.9.10-ubuntu-arm64")},
{Version: version.MustParseBinary("2.9.11-ubuntu-arm64")},
}
s.PatchValue(&coreos.HostOS, func() coreos.OSType { return coreos.Ubuntu })

gomock.InOrder(
s.toolsFinder.EXPECT().FindAgents(common.FindAgentsParams{
MajorVersion: 2, MinorVersion: 9,
ModelType: state.ModelTypeCAAS,
Arch: wantArch,
}).Return(simpleStreams, nil),
s.registryProvider.EXPECT().Tags("jujud-operator").Return(coretools.Versions{
image.NewImageInfo(version.MustParse("2.9.8")),
Expand All @@ -802,20 +814,21 @@ func (s *modelUpgradeSuite) TestFindToolsCAASReleased(c *gc.C) {
image.NewImageInfo(version.MustParse("2.9.11")),
image.NewImageInfo(version.MustParse("2.9.12")), // skip: it's not released in simplestream yet.
}, nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.9").Return("amd64", nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.10.1").Return("amd64", nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.10").Return("amd64", nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.11").Return("amd64", nil),
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.9").Return([]string{"amd64", "arm64"}, nil),
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.10.1").Return([]string{"amd64", "arm64"}, nil),
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.10").Return([]string{"amd64", "arm64"}, nil),
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.11").Return([]string{"amd64", "arm64"}, nil),
s.registryProvider.EXPECT().Close().Return(nil),
)

result, err := api.FindAgents(common.FindAgentsParams{MajorVersion: 2, MinorVersion: 9, ModelType: state.ModelTypeCAAS})
result, err := api.FindAgents(common.FindAgentsParams{
MajorVersion: 2, MinorVersion: 9, ModelType: state.ModelTypeCAAS, Arch: wantArch})
c.Assert(err, jc.ErrorIsNil)
c.Assert(result, gc.DeepEquals, coretools.Versions{
&coretools.Tools{Version: version.MustParseBinary("2.9.9-ubuntu-amd64")},
&coretools.Tools{Version: version.MustParseBinary("2.9.10.1-ubuntu-amd64")},
&coretools.Tools{Version: version.MustParseBinary("2.9.10-ubuntu-amd64")},
&coretools.Tools{Version: version.MustParseBinary("2.9.11-ubuntu-amd64")},
&coretools.Tools{Version: version.MustParseBinary("2.9.9-ubuntu-" + expectArch)},
&coretools.Tools{Version: version.MustParseBinary("2.9.10.1-ubuntu-" + expectArch)},
&coretools.Tools{Version: version.MustParseBinary("2.9.10-ubuntu-" + expectArch)},
&coretools.Tools{Version: version.MustParseBinary("2.9.11-ubuntu-" + expectArch)},
})
}

Expand Down Expand Up @@ -845,7 +858,7 @@ func (s *modelUpgradeSuite) TestFindToolsCAASReleasedExact(c *gc.C) {
image.NewImageInfo(version.MustParse("2.9.11")),
image.NewImageInfo(version.MustParse("2.9.12")),
}, nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.10").Return("amd64", nil),
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.10").Return([]string{"amd64"}, nil),
s.registryProvider.EXPECT().Close().Return(nil),
)

Expand Down Expand Up @@ -887,11 +900,11 @@ func (s *modelUpgradeSuite) TestFindToolsCAASNonReleased(c *gc.C) {
image.NewImageInfo(version.MustParse("2.9.12")),
image.NewImageInfo(version.MustParse("2.9.13")), // skip: it's not released in simplestream yet.
}, nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.9").Return("amd64", nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.10.1").Return("amd64", nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.10").Return("amd64", nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.11").Return("amd64", nil),
s.registryProvider.EXPECT().GetArchitecture("jujud-operator", "2.9.12").Return("", errors.NotFoundf("2.9.12")), // This can only happen on a non-official registry account.
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.9").Return([]string{"amd64", "arm64"}, nil),
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.10.1").Return([]string{"amd64", "arm64"}, nil),
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.10").Return([]string{"amd64", "arm64"}, nil),
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.11").Return([]string{"amd64", "arm64"}, nil),
s.registryProvider.EXPECT().GetArchitectures("jujud-operator", "2.9.12").Return(nil, errors.NotFoundf("2.9.12")), // This can only happen on a non-official registry account.
s.registryProvider.EXPECT().Close().Return(nil),
)

Expand Down
12 changes: 6 additions & 6 deletions docker/registry/internal/acr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,16 @@ func normalizeRepoDetailsAzure(repoDetails *docker.ImageRepoDetails) error {
return nil
}

func (c *azureContainerRegistry) String() string {
func (c azureContainerRegistry) String() string {
return "azurecr.io"
}

// Match checks if the repository details matches current provider format.
func (c *azureContainerRegistry) Match() bool {
func (c azureContainerRegistry) Match() bool {
return strings.Contains(c.repoDetails.ServerAddress, "azurecr.io")
}

func (c *azureContainerRegistry) WrapTransport(...TransportWrapper) error {
func (c azureContainerRegistry) WrapTransport(...TransportWrapper) error {
if c.repoDetails.BasicAuthConfig.Empty() {
return errors.NewNotValid(nil, fmt.Sprintf(`username and password are required for registry %q`, c.repoDetails.Repository))
}
Expand All @@ -57,9 +57,9 @@ func (c azureContainerRegistry) Tags(imageName string) (versions tools.Versions,
return c.fetchTags(url, &response)
}

// GetArchitecture returns the archtecture of the image for the specified tag.
func (c azureContainerRegistry) GetArchitecture(imageName, tag string) (string, error) {
return getArchitecture(imageName, tag, c)
// GetArchitectures returns the architectures of the image for the specified tag.
func (c azureContainerRegistry) GetArchitectures(imageName, tag string) ([]string, error) {
return getArchitectures(imageName, tag, c)
}

// GetManifests returns the manifests of the image for the specified tag.
Expand Down
21 changes: 19 additions & 2 deletions docker/registry/internal/acr_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,10 @@ func (s *azureContainerRegistrySuite) assertGetManifestsSchemaVersion1(c *gc.C,
func (s *azureContainerRegistrySuite) TestGetManifestsSchemaVersion1(c *gc.C) {
s.assertGetManifestsSchemaVersion1(c,
`
{ "schemaVersion": 1, "name": "jujuqa/jujud-operator", "tag": "2.9.13", "architecture": "amd64"}
{ "schemaVersion": 1, "name": "jujuqa/jujud-operator", "tag": "2.9.13", "architecture": "ppc64le"}
`[1:],
`application/vnd.docker.distribution.manifest.v1+prettyjws`,
&internal.ManifestsResult{Architecture: "amd64"},
&internal.ManifestsResult{Architectures: []string{"ppc64el"}},
)
}

Expand All @@ -319,6 +319,23 @@ func (s *azureContainerRegistrySuite) TestGetManifestsSchemaVersion2(c *gc.C) {
)
}

func (s *azureContainerRegistrySuite) TestGetManifestsSchemaVersion2List(c *gc.C) {
s.assertGetManifestsSchemaVersion1(c,
`
{
"schemaVersion": 2,
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"manifests": [
{"platform": {"architecture": "amd64"}},
{"platform": {"architecture": "ppc64le"}}
]
}
`[1:],
`application/vnd.docker.distribution.manifest.list.v2+prettyjws`,
&internal.ManifestsResult{Architectures: []string{"amd64", "ppc64el"}},
)
}

func (s *azureContainerRegistrySuite) TestGetBlobs(c *gc.C) {
// Use v2 for private repository.
s.isPrivate = true
Expand Down
8 changes: 4 additions & 4 deletions docker/registry/internal/base_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,12 @@ func commonURLGetter(version APIVersion, url url.URL, pathTemplate string, args
return url.String()
}

func (c baseClient) url(pathTemplate string, args ...interface{}) string {
func (c *baseClient) url(pathTemplate string, args ...interface{}) string {
return commonURLGetter(c.APIVersion(), *c.baseURL, pathTemplate, args...)
}

// Ping pings the baseClient endpoint.
func (c baseClient) Ping() error {
func (c *baseClient) Ping() error {
url := c.url("/")
logger.Debugf("baseClient ping %q", url)
resp, err := c.client.Get(url)
Expand All @@ -209,7 +209,7 @@ func (c baseClient) Ping() error {
return errors.Trace(unwrapNetError(err))
}

func (c baseClient) ImageRepoDetails() (o docker.ImageRepoDetails) {
func (c *baseClient) ImageRepoDetails() (o docker.ImageRepoDetails) {
if c.repoDetails != nil {
return *c.repoDetails
}
Expand All @@ -224,7 +224,7 @@ func (c *baseClient) Close() error {
return nil
}

func (c baseClient) getPaginatedJSON(url string, response interface{}) (string, error) {
func (c *baseClient) getPaginatedJSON(url string, response interface{}) (string, error) {
resp, err := c.client.Get(url)
logger.Tracef("getPaginatedJSON for %q, err %v", url, err)
if err != nil {
Expand Down
Loading

0 comments on commit f4d5b45

Please sign in to comment.