Skip to content

Commit

Permalink
Merge pull request juju#17194 from jack-w-shaw/JUJU-4948_series_to_ba…
Browse files Browse the repository at this point in the history
…se_in_packaging

juju#17194

juju/packaging v3 no longer uses series in a bunch of places

See the changes to juju/packaging here:
juju/packaging#22

Fix places where we use function from packagings which no taek a series
param

This drops our external dependency on series, so start pushing bases to
replace series up the stack

In most places, we're blocked by HostSeries. We are soon to replace this
with HostBase, which would elimate series entirely in a bunch of places

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing

## QA steps

```
$ juju bootstrap aws aws
$ juju add-model m
$ juju add-machine
$ juju add-machine lxd:0
$ juju add-machine lxd:0 --base [email protected]
(wait)
$ juju status
Model Controller Cloud/Region Version SLA Timestamp
m aws aws/eu-west-2 3.5-beta1.1 unsupported 13:25:13+01:00

Machine State Address Inst id Base AZ Message
0 started 18.170.114.179 i-0a1c12a72964544fe [email protected] eu-west-2c running
0/lxd/0 started 252.41.183.122 juju-d23f30-0-lxd-0 [email protected] eu-west-2c Container started
0/lxd/1 started 252.41.183.143 juju-d23f30-0-lxd-1 [email protected] eu-west-2c Container started
```

## Links

**Jira card:** https://warthogs.atlassian.net/browse/JUJU-4948
  • Loading branch information
jujubot authored Apr 17, 2024
2 parents 4f98c2f + 2bf0b84 commit f3197e3
Show file tree
Hide file tree
Showing 25 changed files with 113 additions and 137 deletions.
4 changes: 2 additions & 2 deletions cloudconfig/cloudinit/cloudinit.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"strings"

"github.com/juju/errors"
"github.com/juju/packaging/v2/commands"
"github.com/juju/packaging/v2/config"
"github.com/juju/packaging/v3/commands"
"github.com/juju/packaging/v3/config"
"github.com/juju/utils/v3/shell"
"github.com/juju/utils/v3/ssh"

Expand Down
4 changes: 2 additions & 2 deletions cloudconfig/cloudinit/cloudinit_centos.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"fmt"
"strings"

"github.com/juju/packaging/v2"
"github.com/juju/packaging/v2/config"
"github.com/juju/packaging/v3"
"github.com/juju/packaging/v3/config"
"gopkg.in/yaml.v2"

corenetwork "github.com/juju/juju/core/network"
Expand Down
2 changes: 1 addition & 1 deletion cloudconfig/cloudinit/cloudinit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ package cloudinit_test
import (
"fmt"

"github.com/juju/packaging/v2"
"github.com/juju/packaging/v3"
jc "github.com/juju/testing/checkers"
sshtesting "github.com/juju/utils/v3/ssh/testing"
"golang.org/x/crypto/ssh"
Expand Down
4 changes: 2 additions & 2 deletions cloudconfig/cloudinit/cloudinit_ubuntu.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"strings"

"github.com/juju/errors"
"github.com/juju/packaging/v2"
"github.com/juju/packaging/v2/config"
"github.com/juju/packaging/v3"
"github.com/juju/packaging/v3/config"
"github.com/juju/proxy"
"github.com/juju/utils/v3"
"gopkg.in/yaml.v2"
Expand Down
10 changes: 5 additions & 5 deletions cloudconfig/cloudinit/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ package cloudinit

import (
"github.com/juju/errors"
"github.com/juju/packaging/v2"
"github.com/juju/packaging/v2/commands"
"github.com/juju/packaging/v2/config"
"github.com/juju/packaging/v3"
"github.com/juju/packaging/v3/commands"
"github.com/juju/packaging/v3/config"
"github.com/juju/proxy"
"github.com/juju/utils/v3/shell"
"golang.org/x/crypto/ssh"
Expand Down Expand Up @@ -433,15 +433,15 @@ func New(osname string, opts ...func(*cloudConfig)) (CloudConfig, error) {
jujupackaging.SnapPackageManager: commands.NewSnapPackageCommander(),
}
cfg.pacconfer = map[jujupackaging.PackageManagerName]config.PackagingConfigurer{
jujupackaging.AptPackageManager: config.NewAptPackagingConfigurer(osname),
jujupackaging.AptPackageManager: config.NewAptPackagingConfigurer(),
}
return &ubuntuCloudConfig{cfg}, nil
case os.CentOS:
cfg.paccmder = map[jujupackaging.PackageManagerName]commands.PackageCommander{
jujupackaging.YumPackageManager: commands.NewYumPackageCommander(),
}
cfg.pacconfer = map[jujupackaging.PackageManagerName]config.PackagingConfigurer{
jujupackaging.YumPackageManager: config.NewYumPackagingConfigurer("centos7"),
jujupackaging.YumPackageManager: config.NewYumPackagingConfigurer(),
}
return &centOSCloudConfig{
cloudConfig: cfg,
Expand Down
2 changes: 1 addition & 1 deletion cloudconfig/cloudinit/renderscript_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package cloudinit_test
import (
"regexp"

"github.com/juju/packaging/v2"
"github.com/juju/packaging/v3"
jc "github.com/juju/testing/checkers"
gc "gopkg.in/check.v1"

Expand Down
4 changes: 2 additions & 2 deletions cloudconfig/cloudinit/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"fmt"
"path/filepath"

"github.com/juju/packaging/v2"
"github.com/juju/packaging/v2/config"
"github.com/juju/packaging/v3"
"github.com/juju/packaging/v3/config"
"github.com/juju/utils/v3"

jujupackaging "github.com/juju/juju/packaging"
Expand Down
7 changes: 6 additions & 1 deletion container/kvm/initialisation.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/juju/os/v2/series"

"github.com/juju/juju/container"
"github.com/juju/juju/core/base"
"github.com/juju/juju/core/paths"
"github.com/juju/juju/packaging"
"github.com/juju/juju/packaging/dependency"
Expand Down Expand Up @@ -48,9 +49,13 @@ func ensureDependencies() error {
if err != nil {
return errors.Trace(err)
}
hostBase, err := base.GetBaseFromSeries(hostSeries)
if err != nil {
return errors.Trace(err)
}

dep := dependency.KVM(runtime.GOARCH)
if err = packaging.InstallDependency(dep, hostSeries); err != nil {
if err = packaging.InstallDependency(dep, hostBase); err != nil {
return errors.Trace(err)
}

Expand Down
11 changes: 6 additions & 5 deletions container/kvm/initialisation_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
jc "github.com/juju/testing/checkers"
gc "gopkg.in/check.v1"

"github.com/juju/juju/core/base"
"github.com/juju/juju/core/paths"
"github.com/juju/juju/packaging"
"github.com/juju/juju/packaging/dependency"
Expand Down Expand Up @@ -93,19 +94,19 @@ func (initialisationInternalSuite) TestAutoStartPool(c *gc.C) {
}

func (initialisationInternalSuite) TestRequiredPackagesAMD64(c *gc.C) {
got, err := dependency.KVM("amd64").PackageList("bionic")
got, err := dependency.KVM("amd64").PackageList(base.MustParseBaseFromString("[email protected]"))
c.Assert(err, gc.IsNil)
assertPkgListMatches(c, got, []string{"qemu-kvm", "qemu-utils", "genisoimage", "libvirt-bin"})
assertPkgListMatches(c, got, []string{"qemu-kvm", "qemu-utils", "genisoimage", "libvirt-daemon-system", "libvirt-clients"})
}

func (initialisationInternalSuite) TestRequiredPackagesARM64(c *gc.C) {
got, err := dependency.KVM("arm64").PackageList("bionic")
got, err := dependency.KVM("amd64").PackageList(base.MustParseBaseFromString("[email protected]"))
c.Assert(err, gc.IsNil)
assertPkgListMatches(c, got, []string{"qemu-efi", "qemu-kvm", "qemu-utils", "genisoimage", "libvirt-bin"})
assertPkgListMatches(c, got, []string{"qemu-kvm", "qemu-utils", "genisoimage", "libvirt-daemon-system", "libvirt-clients"})
}

func assertPkgListMatches(c *gc.C, got []packaging.Package, exp []string) {
c.Assert(len(got), gc.Equals, len(exp))
c.Check(len(got), gc.Equals, len(exp))
for i := 0; i < len(got); i++ {
c.Assert(got[i].Name, gc.Equals, exp[i])
}
Expand Down
13 changes: 9 additions & 4 deletions container/lxd/initialisation_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import (

"github.com/juju/errors"
"github.com/juju/os/v2/series"
"github.com/juju/packaging/v2/manager"
"github.com/juju/packaging/v3/manager"
"github.com/juju/proxy"

"github.com/juju/juju/container"
corebase "github.com/juju/juju/core/base"
"github.com/juju/juju/packaging"
"github.com/juju/juju/packaging/dependency"
"github.com/juju/juju/service"
Expand Down Expand Up @@ -71,8 +72,12 @@ func (ci *containerInitialiser) Initialise() (err error) {
if err != nil {
return errors.Trace(err)
}
localBase, err := corebase.GetBaseFromSeries(localSeries)
if err != nil {
return errors.Trace(err)
}

if err := ensureDependencies(ci.lxdSnapChannel, localSeries); err != nil {
if err := ensureDependencies(ci.lxdSnapChannel, localBase); err != nil {
return errors.Trace(err)
}

Expand Down Expand Up @@ -185,7 +190,7 @@ var df = func(path string) (uint64, error) {
}

// ensureDependencies install the required dependencies for running LXD.
func ensureDependencies(lxdSnapChannel, series string) error {
func ensureDependencies(lxdSnapChannel string, base corebase.Base) error {
// If the snap is already installed, check whether the operator asked
// us to use a different channel. If so, switch to it.
if lxdViaSnap() {
Expand All @@ -209,7 +214,7 @@ func ensureDependencies(lxdSnapChannel, series string) error {
return nil
}

if err := packaging.InstallDependency(dependency.LXD(lxdSnapChannel), series); err != nil {
if err := packaging.InstallDependency(dependency.LXD(lxdSnapChannel), base); err != nil {
return errors.Trace(err)
}

Expand Down
6 changes: 3 additions & 3 deletions container/lxd/initialisation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (

lxd "github.com/canonical/lxd/client"
"github.com/canonical/lxd/shared/api"
"github.com/juju/packaging/v2/commands"
"github.com/juju/packaging/v2/manager"
"github.com/juju/packaging/v3/commands"
"github.com/juju/packaging/v3/manager"
"github.com/juju/proxy"
"github.com/juju/testing"
jc "github.com/juju/testing/checkers"
Expand Down Expand Up @@ -171,7 +171,7 @@ func (s *InitialiserSuite) TestLXDAlreadyInitialized(c *gc.C) {
}

func (s *InitialiserSuite) TestInitializeSetsProxies(c *gc.C) {
PatchHostSeries(s, "")
PatchHostSeries(s, "jammy")

ctrl := gomock.NewController(c)
defer ctrl.Finish()
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ require (
github.com/juju/names/v5 v5.0.0
github.com/juju/naturalsort v1.0.0
github.com/juju/os/v2 v2.2.3
github.com/juju/packaging/v2 v2.0.1
github.com/juju/packaging/v3 v3.0.0
github.com/juju/persistent-cookiejar v1.0.0
github.com/juju/proxy v1.0.0
github.com/juju/pubsub/v2 v2.0.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,8 @@ github.com/juju/naturalsort v1.0.0 h1:kGmUUy3h8mJ5/SJYaqKOBR3f3owEd5R52Lh+Tjg/dN
github.com/juju/naturalsort v1.0.0/go.mod h1:Zqa/vGkXr78k47zM6tFmU9phhxKz/PIdqBzpLhJ86zc=
github.com/juju/os/v2 v2.2.3 h1:5SnGWfzFTXcFwu/sd9qEEf/No3UZxivOjJuWmsHI4N4=
github.com/juju/os/v2 v2.2.3/go.mod h1:xGfP9I+Xb/A03NcGBsoJgwr084hPckkQHecaHuV3wBQ=
github.com/juju/packaging/v2 v2.0.1 h1:KeTfqx3Z0c6RcM053GJH7mplroXoRSuh/dK5vqDQLn8=
github.com/juju/packaging/v2 v2.0.1/go.mod h1:JC+FIRTJXGLt9wA+iP3ltkzv+aWVMMojB/R47uIAK0Y=
github.com/juju/packaging/v3 v3.0.0 h1:ZzuHhR8Ql9z2oeQ0m73x6k58PW65Cgk5wR9Yc1exoOI=
github.com/juju/packaging/v3 v3.0.0/go.mod h1:WXh/SXqh1du8SFzwb1KC+yZuV4Qc4alWP3MEPqFX9Lw=
github.com/juju/persistent-cookiejar v1.0.0 h1:Ag7+QLzqC2m+OYXy2QQnRjb3gTkEBSZagZ6QozwT3EQ=
github.com/juju/persistent-cookiejar v1.0.0/go.mod h1:zrbmo4nBKaiP/Ez3F67ewkMbzGYfXyMvRtbOfuAwG0w=
github.com/juju/postgrestest v1.1.0/go.mod h1:/n17Y2T6iFozzXwSCO0JYJ5gSiz2caEtSwAjh/uLXDM=
Expand Down
3 changes: 2 additions & 1 deletion mongo/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
jc "github.com/juju/testing/checkers"
gc "gopkg.in/check.v1"

"github.com/juju/juju/core/base"
"github.com/juju/juju/mongo"
"github.com/juju/juju/packaging"
coretesting "github.com/juju/juju/testing"
Expand All @@ -23,7 +24,7 @@ var _ = gc.Suite(&adminSuite{})

func (s *adminSuite) SetUpTest(c *gc.C) {
s.BaseSuite.SetUpTest(c)
s.PatchValue(mongo.InstallMongo, func(dep packaging.Dependency, series string) error {
s.PatchValue(mongo.InstallMongo, func(dep packaging.Dependency, b base.Base) error {
return nil
})
}
Expand Down
11 changes: 8 additions & 3 deletions mongo/mongo.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/juju/retry"
"github.com/juju/utils/v3"

"github.com/juju/juju/core/base"
"github.com/juju/juju/core/network"
"github.com/juju/juju/packaging"
"github.com/juju/juju/packaging/dependency"
Expand Down Expand Up @@ -228,6 +229,10 @@ func ensureServer(ctx context.Context, args EnsureServerParams, mongoKernelTweak
if err != nil {
return errors.Annotatef(err, "cannot get host series")
}
hostBase, err := base.GetBaseFromSeries(hostSeries)
if err != nil {
return errors.Annotatef(err, "host series %q not a vlaid base", hostSeries)
}

mongoDep := dependency.Mongo(args.JujuDBSnapChannel)
if args.DataDir == "" {
Expand All @@ -252,7 +257,7 @@ func ensureServer(ctx context.Context, args EnsureServerParams, mongoKernelTweak
return errors.Annotatef(err, "cannot create mongo snap service")
}

if err := installMongod(mongoDep, hostSeries, svc); err != nil {
if err := installMongod(mongoDep, hostBase, svc); err != nil {
return errors.Annotatef(err, "cannot install mongod")
}

Expand Down Expand Up @@ -455,11 +460,11 @@ func mongoSnapService(dataDir, configDir, snapChannel string) (MongoSnapService,
// Override for testing.
var installMongo = packaging.InstallDependency

func installMongod(mongoDep packaging.Dependency, hostSeries string, snapSvc MongoSnapService) error {
func installMongod(mongoDep packaging.Dependency, hostBase base.Base, snapSvc MongoSnapService) error {
// Do either a local snap install or a real install from the store.
if snapSvc.Name() == ServiceName {
// Store snap.
return installMongo(mongoDep, hostSeries)
return installMongo(mongoDep, hostBase)
} else {
// Local snap.
return snapSvc.Install()
Expand Down
3 changes: 2 additions & 1 deletion mongo/mongo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"go.uber.org/mock/gomock"
gc "gopkg.in/check.v1"

"github.com/juju/juju/core/base"
"github.com/juju/juju/core/network"
"github.com/juju/juju/mongo"
"github.com/juju/juju/mongo/mongotest"
Expand Down Expand Up @@ -226,7 +227,7 @@ func (s *MongoSuite) TestEnsureServerInstalledError(c *gc.C) {
testing.PatchExecutableAsEchoArgs(c, s, "snap")

failure := errors.New("boom")
s.PatchValue(mongo.InstallMongo, func(dep packaging.Dependency, series string) error {
s.PatchValue(mongo.InstallMongo, func(dep packaging.Dependency, b base.Base) error {
return failure
})

Expand Down
23 changes: 9 additions & 14 deletions packaging/dependency/kvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ type kvmDependency struct {
}

// PackageList implements packaging.Dependency.
func (dep kvmDependency) PackageList(series string) ([]packaging.Package, error) {
if series == base.Centos7.String() || series == base.Centos9.String() {
return nil, errors.NotSupportedf("installing kvm on series %q", series)
func (dep kvmDependency) PackageList(b base.Base) ([]packaging.Package, error) {
if b.OS != ubuntu {
return nil, errors.NotSupportedf("installing kvm on base %q", b)
}

var pkgList []string
Expand All @@ -39,17 +39,12 @@ func (dep kvmDependency) PackageList(series string) ([]packaging.Package, error)
"genisoimage",
)

switch series {
case "bionic":
pkgList = append(pkgList, "libvirt-bin")
default:
// On focal+ virsh is provided by libvirt-clients; also we need
// to install the daemon package separately.
pkgList = append(pkgList,
"libvirt-daemon-system",
"libvirt-clients",
)
}
// On focal+ virsh is provided by libvirt-clients; also we need
// to install the daemon package separately.
pkgList = append(pkgList,
"libvirt-daemon-system",
"libvirt-clients",
)

return packaging.MakePackageList(packaging.AptPackageManager, "", pkgList...), nil
}
28 changes: 12 additions & 16 deletions packaging/dependency/lxd.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/juju/juju/packaging"
)

const blankSeries = ""
var ubuntu = "ubuntu"

// LXD returns a dependency instance for installing lxd support using the
// specified channel preferences (applies to cosmic or later).
Expand All @@ -27,24 +27,20 @@ type lxdDependency struct {
}

// PackageList implements packaging.Dependency.
func (dep lxdDependency) PackageList(series string) ([]packaging.Package, error) {
func (dep lxdDependency) PackageList(b base.Base) ([]packaging.Package, error) {
if b.OS != ubuntu {
return nil, errors.NotSupportedf("LXD containers on base %q", b)
}

var pkg packaging.Package

switch series {
case base.Centos7.String(), base.Centos9.String():
return nil, errors.NotSupportedf("LXD containers on series %q", series)
case base.Bionic.String(), blankSeries:
pkg.Name = "lxd"
pkg.PackageManager = packaging.AptPackageManager
default: // Use snaps for cosmic and beyond
if dep.snapChannel == "" {
return nil, errors.NotValidf("snap channel for lxd dependency")
}

pkg.Name = "lxd"
pkg.InstallOptions = fmt.Sprintf("--classic --channel %s", dep.snapChannel)
pkg.PackageManager = packaging.SnapPackageManager
if dep.snapChannel == "" {
return nil, errors.NotValidf("snap channel for lxd dependency")
}

pkg.Name = "lxd"
pkg.InstallOptions = fmt.Sprintf("--classic --channel %s", dep.snapChannel)
pkg.PackageManager = packaging.SnapPackageManager

return []packaging.Package{pkg}, nil
}
Loading

0 comments on commit f3197e3

Please sign in to comment.