Skip to content

Commit

Permalink
Fix parsing of custom k8s image repos with a port
Browse files Browse the repository at this point in the history
  • Loading branch information
wallyworld committed Jul 7, 2021
1 parent 43b8197 commit fe8bdf7
Show file tree
Hide file tree
Showing 13 changed files with 99 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,10 @@ func (a *API) provisioningInfo(appName names.ApplicationTag) (*params.CAASApplic
fmt.Sprintf("agent version is missing in model config %q", modelConfig.Name()),
)
}
imagePath := podcfg.GetJujuOCIImagePath(cfg, vers.ToPatch(), version.OfficialBuild)
imagePath, err := podcfg.GetJujuOCIImagePath(cfg, vers.ToPatch(), version.OfficialBuild)
if err != nil {
return nil, errors.Trace(err)
}

apiHostPorts, err := a.ctrlSt.APIHostPortsForAgents()
if err != nil {
Expand Down
10 changes: 7 additions & 3 deletions apiserver/facades/controller/caasmodeloperator/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,15 @@ func (a *API) ModelOperatorProvisioningInfo() (params.ModelOperatorInfo, error)
return result, errors.Annotate(err, "getting api addresses")
}

imagePath, err := podcfg.GetJujuOCIImagePath(controllerConf,
vers.ToPatch(), version.OfficialBuild)
if err != nil {
return result, errors.Trace(err)
}
result = params.ModelOperatorInfo{
APIAddresses: apiAddresses.Result,
ImagePath: podcfg.GetJujuOCIImagePath(controllerConf,
vers.ToPatch(), version.OfficialBuild),
Version: vers,
ImagePath: imagePath,
Version: vers,
}
return result, nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ func (m *ModelOperatorSuite) TestProvisioningInfo(c *gc.C) {
controllerConf, err := m.state.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

imagePath := podcfg.GetJujuOCIImagePath(controllerConf, info.Version, version.OfficialBuild)
imagePath, err := podcfg.GetJujuOCIImagePath(controllerConf, info.Version, version.OfficialBuild)
c.Assert(err, jc.ErrorIsNil)
c.Assert(imagePath, gc.Equals, info.ImagePath)

model, err := m.state.Model()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,10 @@ func (a *API) OperatorProvisioningInfo(args params.Entities) (params.OperatorPro
modelConfig,
)

imagePath := podcfg.GetJujuOCIImagePath(cfg, vers.ToPatch(), version.OfficialBuild)
imagePath, err := podcfg.GetJujuOCIImagePath(cfg, vers.ToPatch(), version.OfficialBuild)
if err != nil {
return result, errors.Trace(err)
}
apiAddresses, err := a.APIAddresses()
if err == nil && apiAddresses.Error != nil {
err = apiAddresses.Error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,10 @@ func (f *Facade) provisioningInfo(model Model, tagString string) (*params.Kubern
fmt.Sprintf("agent version is missing in model config %q", modelConfig.Name()),
)
}
operatorImagePath := podcfg.GetJujuOCIImagePath(controllerCfg, vers.ToPatch(), version.OfficialBuild)
operatorImagePath, err := podcfg.GetJujuOCIImagePath(controllerCfg, vers.ToPatch(), version.OfficialBuild)
if err != nil {
return nil, errors.Trace(err)
}

filesystemParams, err := f.applicationFilesystemParams(app, controllerCfg, modelConfig)
if err != nil {
Expand Down
6 changes: 5 additions & 1 deletion caas/kubernetes/provider/application/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,11 @@ func (a *app) upgradeMainResource(applier resources.Applier, ver version.Number)
return errors.NotValidf("init container of %q", a.name)
}
initContainer := initContainers[0]
initContainer.Image = podcfg.RebuildOldOperatorImagePath(initContainer.Image, ver)
var err error
initContainer.Image, err = podcfg.RebuildOldOperatorImagePath(initContainer.Image, ver)
if err != nil {
return errors.Trace(err)
}
ss.Spec.Template.Spec.InitContainers = []corev1.Container{initContainer}
ss.Spec.Template.SetAnnotations(a.upgradeAnnotations(annotations.New(ss.Spec.Template.GetAnnotations()), ver))
ss.SetAnnotations(a.upgradeAnnotations(annotations.New(ss.GetAnnotations()), ver))
Expand Down
16 changes: 12 additions & 4 deletions caas/kubernetes/provider/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,7 @@ func (c *controllerStack) buildContainerSpecForController(statefulset *apps.Stat
if c.pcfg.Controller.Config.MongoMemoryProfile() == string(mongo.MemoryProfileLow) {
wiredTigerCacheSize = mongo.Mongo34LowCacheSize
}
generateContainerSpecs := func(jujudCmd string) []core.Container {
generateContainerSpecs := func(jujudCmd string) ([]core.Container, error) {
var containerSpec []core.Container
// add container mongoDB.
// TODO(bootstrap): refactor mongo package to make it usable for IAAS and CAAS,
Expand Down Expand Up @@ -1194,10 +1194,14 @@ func (c *controllerStack) buildContainerSpecForController(statefulset *apps.Stat
})

// add container API server.
controllerImage, err := c.pcfg.GetControllerImagePath()
if err != nil {
return nil, errors.Trace(err)
}
containerSpec = append(containerSpec, core.Container{
Name: apiServerContainerName,
ImagePullPolicy: core.PullIfNotPresent,
Image: c.pcfg.GetControllerImagePath(),
Image: controllerImage,
Command: []string{
"/bin/sh",
},
Expand Down Expand Up @@ -1247,7 +1251,7 @@ func (c *controllerStack) buildContainerSpecForController(statefulset *apps.Stat
},
})
c.containerCount = len(containerSpec)
return containerSpec
return containerSpec, nil
}

loggingOption := "--show-log"
Expand Down Expand Up @@ -1285,7 +1289,11 @@ func (c *controllerStack) buildContainerSpecForController(statefulset *apps.Stat
c.pcfg.ControllerId,
loggingOption,
)
statefulset.Spec.Template.Spec.Containers = generateContainerSpecs(jujudCmd)
containers, err := generateContainerSpecs(jujudCmd)
if err != nil {
return errors.Trace(err)
}
statefulset.Spec.Template.Spec.Containers = containers
return nil
}

Expand Down
5 changes: 4 additions & 1 deletion caas/kubernetes/provider/operator_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,10 @@ func operatorUpgrade(
return errors.Trace(err)
}

operatorImagePath := podcfg.RebuildOldOperatorImagePath(operator.Config.OperatorImagePath, vers)
operatorImagePath, err := podcfg.RebuildOldOperatorImagePath(operator.Config.OperatorImagePath, vers)
if err != nil {
return errors.Trace(err)
}
if len(operatorImagePath) == 0 {
// This should never happen.
return errors.NotValidf("no resource is upgradable for application %q", appName)
Expand Down
6 changes: 5 additions & 1 deletion caas/kubernetes/provider/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,12 @@ func upgradePodTemplateSpec(
}

if imagePath == "" {
imagePath = podcfg.RebuildOldOperatorImagePath(
var err error
imagePath, err = podcfg.RebuildOldOperatorImagePath(
podTemplate.Spec.Containers[jujudContainerIdx].Image, vers)
if err != nil {
return nil, errors.Trace(err)
}
}

upgradedTemp := podTemplate.DeepCopy()
Expand Down
48 changes: 24 additions & 24 deletions cloudconfig/podcfg/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"strings"

"github.com/docker/distribution/reference"
"github.com/juju/charm/v8"
"github.com/juju/errors"
"github.com/juju/version/v2"
Expand All @@ -15,14 +16,13 @@ import (
)

const (
JujudOCINamespace = "jujusolutions"
JujudOCIName = "jujud-operator"
JujuContainerAgentName = "containeragent"
JujudbOCIName = "juju-db"
JujudOCINamespace = "jujusolutions"
JujudOCIName = "jujud-operator"
JujudbOCIName = "juju-db"
)

// GetControllerImagePath returns oci image path of jujud for a controller.
func (cfg *ControllerPodConfig) GetControllerImagePath() string {
func (cfg *ControllerPodConfig) GetControllerImagePath() (string, error) {
return GetJujuOCIImagePath(cfg.Controller.Config, cfg.JujuVersion, cfg.OfficialBuild)
}

Expand All @@ -42,44 +42,44 @@ func IsJujuOCIImage(imagePath string) bool {
}

// GetJujuOCIImagePath returns the jujud oci image path.
func GetJujuOCIImagePath(controllerCfg controller.Config, ver version.Number, build int) string {
func GetJujuOCIImagePath(controllerCfg controller.Config, ver version.Number, build int) (string, error) {
// First check the deprecated "caas-operator-image-path" config.
ver.Build = build
imagePath := RebuildOldOperatorImagePath(
imagePath, err := RebuildOldOperatorImagePath(
controllerCfg.CAASOperatorImagePath(), ver,
)
if imagePath != "" {
return imagePath
if imagePath != "" || err != nil {
return imagePath, err
}
return imageRepoToPath(controllerCfg.CAASImageRepo(), ver)
}

// RebuildOldOperatorImagePath returns a updated image path for the specified juju version.
func RebuildOldOperatorImagePath(imagePath string, ver version.Number) string {
func RebuildOldOperatorImagePath(imagePath string, ver version.Number) (string, error) {
if imagePath == "" {
return ""
return "", nil
}
return tagImagePath(imagePath, ver)
}

func tagImagePath(path string, ver version.Number) string {
var verString string
splittedPath := strings.Split(path, ":")
path = splittedPath[0]
if len(splittedPath) > 1 {
verString = splittedPath[1]
func tagImagePath(fullPath string, ver version.Number) (string, error) {
ref, err := reference.Parse(fullPath)
if err != nil {
return "", errors.Trace(err)
}
if ver != version.Zero {
verString = ver.String()
imageNamed, ok := ref.(reference.Named)
// Safety check only - should never happen.
if !ok {
return "", errors.Errorf("unexpected docker image path type, got %T, expected reference.Named", ref)
}
if verString != "" {
// tag with version.
path += ":" + verString
if ver != version.Zero {
// ver is always a valid tag.
imageNamed, _ = reference.WithTag(imageNamed, ver.String())
}
return path
return imageNamed.String(), nil
}

func imageRepoToPath(imageRepo string, ver version.Number) string {
func imageRepoToPath(imageRepo string, ver version.Number) (string, error) {
if imageRepo == "" {
imageRepo = JujudOCINamespace
}
Expand Down
19 changes: 17 additions & 2 deletions cloudconfig/podcfg/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,29 @@ func (*imageSuite) TestGetJujuOCIImagePath(c *gc.C) {

cfg[controller.CAASImageRepo] = "testing-repo"
ver := version.MustParse("2.6-beta3")
path := podcfg.GetJujuOCIImagePath(cfg, ver, 666)
path, err := podcfg.GetJujuOCIImagePath(cfg, ver, 666)
c.Assert(err, jc.ErrorIsNil)
c.Assert(path, jc.DeepEquals, "testing-repo/jujud-operator:2.6-beta3.666")

cfg[controller.CAASImageRepo] = "testing-repo:8080"
ver = version.MustParse("2.6-beta3")
path, err = podcfg.GetJujuOCIImagePath(cfg, ver, 666)
c.Assert(err, jc.ErrorIsNil)
c.Assert(path, jc.DeepEquals, "testing-repo:8080/jujud-operator:2.6-beta3.666")

cfg[controller.CAASOperatorImagePath] = "testing-old-repo/jujud-old-operator:1.6"
path = podcfg.GetJujuOCIImagePath(cfg, ver, 0)
path, err = podcfg.GetJujuOCIImagePath(cfg, ver, 0)
c.Assert(err, jc.ErrorIsNil)
c.Assert(path, jc.DeepEquals, "testing-old-repo/jujud-old-operator:2.6-beta3")
}

func (*imageSuite) TestRebuildOldOperatorImagePath(c *gc.C) {
ver := version.MustParse("2.6-beta3")
path, err := podcfg.RebuildOldOperatorImagePath("jujusolutions/jujud-operator:666", ver)
c.Assert(err, jc.ErrorIsNil)
c.Assert(path, jc.DeepEquals, "jujusolutions/jujud-operator:2.6-beta3")
}

func (*imageSuite) TestImageForBase(c *gc.C) {
_, err := podcfg.ImageForBase("", charm.Base{})
c.Assert(err, gc.ErrorMatches, `empty base name not valid`)
Expand Down
9 changes: 7 additions & 2 deletions cloudconfig/podcfg/podcfg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ func (*podcfgSuite) TestOperatorImagesDefaultRepo(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
podConfig.JujuVersion = version.MustParse("6.6.6")
podConfig.OfficialBuild = 666
c.Assert(podConfig.GetControllerImagePath(), gc.Equals, "jujusolutions/jujud-operator:6.6.6.666")
path, err := podConfig.GetControllerImagePath()
c.Assert(err, jc.ErrorIsNil)
c.Assert(path, gc.Equals, "jujusolutions/jujud-operator:6.6.6.666")
c.Assert(podConfig.GetJujuDbOCIImagePath(), gc.Equals, "jujusolutions/juju-db:4.0")
}

Expand All @@ -82,7 +84,10 @@ func (*podcfgSuite) TestOperatorImagesCustomRepo(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
podConfig.JujuVersion = version.MustParse("6.6.6")
podConfig.OfficialBuild = 666
c.Assert(podConfig.GetControllerImagePath(), gc.Equals, "path/to/my/repo/jujud-operator:6.6.6.666")
c.Assert(err, jc.ErrorIsNil)
path, err := podConfig.GetControllerImagePath()
c.Assert(err, jc.ErrorIsNil)
c.Assert(path, gc.Equals, "path/to/my/repo/jujud-operator:6.6.6.666")
c.Assert(podConfig.GetJujuDbOCIImagePath(), gc.Equals, "path/to/my/repo/juju-db:4.0")
}

Expand Down
5 changes: 4 additions & 1 deletion cmd/juju/commands/upgradecontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,10 @@ func (c *baseUpgradeCommand) initCAASVersions(
controllerCfg controller.Config, majorVersion int, streamsAgents tools.List,
) (tools.Versions, error) {
logger.Debugf("searching for agent images with major: %d", majorVersion)
imagePath := podcfg.GetJujuOCIImagePath(controllerCfg, version.Zero, 0)
imagePath, err := podcfg.GetJujuOCIImagePath(controllerCfg, version.Zero, 0)
if err != nil {
return nil, errors.Trace(err)
}
availableTags, err := docker.ListOperatorImages(imagePath)
if err != nil {
return nil, err
Expand Down

0 comments on commit fe8bdf7

Please sign in to comment.