Skip to content

Commit

Permalink
Merge pull request juju#16661 from SimonRichardson/bootstrap-agent-tools
Browse files Browse the repository at this point in the history
juju#16661

~~Requires juju#16647 to land.~~

----

Moves the bootstrap prepareTools that was previously run by the
cmd/agent/bootstrap state command, to the bootstrap worker. The
strategy is actually in the internal package, which gives us
freedom to implement the strategy based on the dependencies. The
uploading of the tools only happens on in IAAS models. There is no
if isCAAS check, it's up to the dependency engine to give us the right
dependencies. The code path becomes branchless on that dependency.

The userdataconfig removed the tools binary after it ran, this was
problematic as the worker isn't guaranteed to have run with in that
window. Removing that code and pushing it into the strategy now ensures
it correctly cleans up it's own concern. If the upload fails, the
tools file will still be there, allowing us to retry later.

Lastly, the big major change is to run the bootstrap worker only if
the bootstrap-params file is available. Only once the bootstrap worker
has completed do we remove that file. This should allow us to perform
retries at a later time if we want to make bootstrapping more reliable.

## 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

```shj
$ juju bootstrap lxd test --build-agent
$ juju enable-ha
$ juju add-model default
$ juju deploy ubuntu
```

## Links

**Jira card:** JUJU-5136
  • Loading branch information
jujubot authored Dec 5, 2023
2 parents 94a3591 + 97996d7 commit 8308806
Show file tree
Hide file tree
Showing 25 changed files with 1,014 additions and 302 deletions.
4 changes: 2 additions & 2 deletions agent/tools/toolsdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ func ReadTools(dataDir string, vers version.Binary) (*coretools.Tools, error) {
dir := SharedToolsDir(dataDir, vers)
toolsData, err := os.ReadFile(path.Join(dir, toolsFile))
if err != nil {
return nil, fmt.Errorf("cannot read agent metadata in directory %v: %v", dir, err)
return nil, fmt.Errorf("cannot read agent metadata in directory %v: %w", dir, err)
}
var tools coretools.Tools
if err := json.Unmarshal(toolsData, &tools); err != nil {
return nil, fmt.Errorf("invalid agent metadata in directory %q: %v", dir, err)
return nil, fmt.Errorf("invalid agent metadata in directory %q: %w", dir, err)
}
return &tools, nil
}
Expand Down
61 changes: 0 additions & 61 deletions cmd/jujud/agent/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
package agent

import (
"bytes"
"context"
stdcontext "context"
"fmt"
"io"
Expand All @@ -22,12 +20,10 @@ import (
"github.com/juju/loggo"
"github.com/juju/names/v4"
"github.com/juju/utils/v3/ssh"
"github.com/juju/version/v2"

"github.com/juju/juju/agent"
"github.com/juju/juju/agent/agentbootstrap"
agentconfig "github.com/juju/juju/agent/config"
agenttools "github.com/juju/juju/agent/tools"
"github.com/juju/juju/caas"
k8sprovider "github.com/juju/juju/caas/kubernetes/provider"
k8sconstants "github.com/juju/juju/caas/kubernetes/provider/constants"
Expand Down Expand Up @@ -55,7 +51,6 @@ import (
"github.com/juju/juju/internal/objectstore"
"github.com/juju/juju/internal/tools"
"github.com/juju/juju/state"
"github.com/juju/juju/state/binarystorage"
"github.com/juju/juju/state/cloudimagemetadata"
"github.com/juju/juju/state/stateenvirons"
jujuversion "github.com/juju/juju/version"
Expand Down Expand Up @@ -427,10 +422,6 @@ func (c *BootstrapCommand) Run(ctx *cmd.Context) error {
}

if !isCAAS {
// Populate the tools catalogue.
if err := c.populateTools(ctx, st, args.ControllerConfig); err != nil {
return errors.Trace(err)
}
// Add custom image metadata to environment storage.
if len(args.CustomImageMetadata) > 0 {
if err := c.saveCustomImageMetadata(st, env, args.CustomImageMetadata); err != nil {
Expand Down Expand Up @@ -574,58 +565,6 @@ func (c *BootstrapCommand) startMongo(ctx stdcontext.Context, isCAAS bool, addrs
return nil
}

// populateTools stores uploaded tools in provider storage
// and updates the tools metadata.
func (c *BootstrapCommand) populateTools(ctx context.Context, st *state.State, controllerConfig controller.Config) error {
agentConfig := c.CurrentConfig()
dataDir := agentConfig.DataDir()

current := version.Binary{
Number: jujuversion.Current,
Arch: arch.HostArch(),
Release: coreos.HostOSTypeName(),
}
agentTools, err := agenttools.ReadTools(dataDir, current)
if err != nil {
return errors.Trace(err)
}

data, err := os.ReadFile(filepath.Join(
agenttools.SharedToolsDir(dataDir, current),
"tools.tar.gz",
))
if err != nil {
return errors.Trace(err)
}

objectStore, err := objectstore.ObjectStoreFactory(ctx,
objectstore.BackendTypeOrDefault(controllerConfig.ObjectStoreType()),
st.ControllerModelUUID(),
objectstore.WithMongoSession(st),
objectstore.WithLogger(logger),
)
if err != nil {
return errors.Trace(err)
}

toolStorage, err := st.ToolsStorage(objectStore)
if err != nil {
return errors.Trace(err)
}
defer func() { _ = toolStorage.Close() }()

metadata := binarystorage.Metadata{
Version: agentTools.Version.String(),
Size: agentTools.Size,
SHA256: agentTools.SHA256,
}
logger.Debugf("Adding agent binary: %v", agentTools.Version)
if err := toolStorage.Add(ctx, bytes.NewReader(data), metadata); err != nil {
return errors.Trace(err)
}
return nil
}

// saveCustomImageMetadata stores the custom image metadata to the database,
func (c *BootstrapCommand) saveCustomImageMetadata(st *state.State, env environs.BootstrapEnviron, imageMetadata []*imagemetadata.ImageMetadata) error {
logger.Debugf("saving custom image metadata")
Expand Down
45 changes: 0 additions & 45 deletions cmd/jujud/agent/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
jtesting "github.com/juju/testing"
jc "github.com/juju/testing/checkers"
"github.com/juju/utils/v3"
"github.com/juju/version/v2"
"go.uber.org/mock/gomock"
gc "gopkg.in/check.v1"

Expand Down Expand Up @@ -638,50 +637,6 @@ func (s *BootstrapSuite) TestSystemIdentityWritten(c *gc.C) {
c.Assert(string(data), gc.Equals, "private-key")
}

func (s *BootstrapSuite) TestDownloadedToolsMetadata(c *gc.C) {
// Tools downloaded by cloud-init script.
s.testToolsMetadata(c)
}

func (s *BootstrapSuite) TestUploadedToolsMetadata(c *gc.C) {
// Tools uploaded over ssh.
s.writeDownloadedTools(c, &tools.Tools{
Version: testing.CurrentVersion(),
URL: "file:///does/not/matter",
})
s.testToolsMetadata(c)
}

func (s *BootstrapSuite) testToolsMetadata(c *gc.C) {
envtesting.RemoveFakeToolsMetadata(c, s.toolsStorage)

_, cmd, err := s.initBootstrapCommand(c, nil)

c.Assert(err, jc.ErrorIsNil)
err = cmd.Run(cmdtesting.Context(c))
c.Assert(err, jc.ErrorIsNil)

// We don't write metadata at bootstrap anymore.
ss := simplestreams.NewSimpleStreams(sstesting.TestDataSourceFactory())
simplestreamsMetadata, err := envtools.ReadMetadata(ss, s.toolsStorage, "released")
c.Assert(err, jc.ErrorIsNil)
c.Assert(simplestreamsMetadata, gc.HasLen, 0)

// The tools should have been added to tools storage.
st, closer := s.getSystemState(c)
defer closer()

storage, err := st.ToolsStorage(jujutesting.NewObjectStore(c, st.ControllerModelUUID(), st))
c.Assert(err, jc.ErrorIsNil)
defer storage.Close()
metadata, err := storage.AllMetadata()
c.Assert(err, jc.ErrorIsNil)
c.Assert(metadata, gc.HasLen, 1)
m := metadata[0]
v := version.MustParseBinary(m.Version)
c.Assert(v.Release, gc.Equals, coreos.HostOSTypeName())
}

func createImageMetadata() []*imagemetadata.ImageMetadata {
return []*imagemetadata.ImageMetadata{{
Id: "imageId",
Expand Down
30 changes: 16 additions & 14 deletions cmd/jujud/agent/machine/manifolds.go
Original file line number Diff line number Diff line change
Expand Up @@ -835,13 +835,14 @@ func IAASManifolds(config ManifoldsConfig) dependency.Manifolds {
manifolds := dependency.Manifolds{
// Bootstrap worker is responsible for setting up the initial machine.
bootstrapName: ifDatabaseUpgradeComplete(bootstrap.Manifold(bootstrap.ManifoldConfig{
AgentName: agentName,
StateName: stateName,
ObjectStoreName: objectStoreName,
BootstrapGateName: isBootstrapGateName,
AgentBinarySeeder: bootstrap.IAASAgentBinarySeeder,
RequiresBootstrap: bootstrap.RequiresBootstrap,
Logger: loggo.GetLogger("juju.worker.bootstrap"),
AgentName: agentName,
StateName: stateName,
ObjectStoreName: objectStoreName,
ServiceFactoryName: serviceFactoryName,
BootstrapGateName: isBootstrapGateName,
AgentBinaryUploader: bootstrap.IAASAgentBinaryUploader,
RequiresBootstrap: bootstrap.RequiresBootstrap,
Logger: loggo.GetLogger("juju.worker.bootstrap"),
})),

toolsVersionCheckerName: ifNotMigrating(toolsversionchecker.Manifold(toolsversionchecker.ManifoldConfig{
Expand Down Expand Up @@ -1051,13 +1052,14 @@ func CAASManifolds(config ManifoldsConfig) dependency.Manifolds {
return mergeManifolds(config, dependency.Manifolds{
// Bootstrap worker is responsible for setting up the initial machine.
bootstrapName: ifDatabaseUpgradeComplete(bootstrap.Manifold(bootstrap.ManifoldConfig{
AgentName: agentName,
StateName: stateName,
ObjectStoreName: objectStoreName,
BootstrapGateName: isBootstrapGateName,
AgentBinarySeeder: bootstrap.CAASAgentBinarySeeder,
RequiresBootstrap: bootstrap.RequiresBootstrap,
Logger: loggo.GetLogger("juju.worker.bootstrap"),
AgentName: agentName,
StateName: stateName,
ObjectStoreName: objectStoreName,
ServiceFactoryName: serviceFactoryName,
BootstrapGateName: isBootstrapGateName,
AgentBinaryUploader: bootstrap.CAASAgentBinaryUploader,
RequiresBootstrap: bootstrap.RequiresBootstrap,
Logger: loggo.GetLogger("juju.worker.bootstrap"),
})),

// TODO(caas) - when we support HA, only want this on primary
Expand Down
2 changes: 1 addition & 1 deletion domain/autocert/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ WHERE name = $M.name`
if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error {
return errors.Trace(tx.Query(ctx, s, sqlair.M{"name": name}).Get(&row))
}); err != nil {
if err.Error() == sql.ErrNoRows.Error() {
if errors.Is(err, sql.ErrNoRows) {
return nil, errors.Annotatef(errors.NotFound, "autocert %s", name)
}
return nil, errors.Annotate(err, "querying autocert cache")
Expand Down
8 changes: 7 additions & 1 deletion domain/flag/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package service

import (
"context"

"github.com/juju/errors"
)

// State describes retrieval and persistence methods for storage.
Expand Down Expand Up @@ -32,5 +34,9 @@ func (s *Service) SetFlag(ctx context.Context, flag string, value bool) error {

// GetFlag returns the value of a flag.
func (s *Service) GetFlag(ctx context.Context, flag string) (bool, error) {
return s.st.GetFlag(ctx, flag)
value, err := s.st.GetFlag(ctx, flag)
if err != nil && !errors.Is(err, errors.NotFound) {
return false, err
}
return value, nil
}
12 changes: 12 additions & 0 deletions domain/flag/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package service
import (
"context"

"github.com/juju/errors"
"github.com/juju/testing"
jc "github.com/juju/testing/checkers"
gomock "go.uber.org/mock/gomock"
Expand Down Expand Up @@ -41,6 +42,17 @@ func (s *serviceSuite) TestGetFlag(c *gc.C) {
c.Check(value, jc.IsTrue)
}

func (s *serviceSuite) TestGetFlagNotFound(c *gc.C) {
defer s.setupMocks(c).Finish()

s.state.EXPECT().GetFlag(gomock.Any(), "foo").Return(false, errors.NotFoundf("flag"))

service := NewService(s.state)
value, err := service.GetFlag(context.Background(), "foo")
c.Assert(err, jc.ErrorIsNil)
c.Check(value, jc.IsFalse)
}

func (s *serviceSuite) setupMocks(c *gc.C) *gomock.Controller {
ctrl := gomock.NewController(c)

Expand Down
3 changes: 3 additions & 0 deletions domain/flag/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ WHERE name = ?;
err = db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
row := tx.QueryRowContext(ctx, query, flag)
if err := row.Scan(&value); err != nil {
if errors.Is(err, sql.ErrNoRows) {
return errors.NotFoundf("flag %q", flag)
}
return errors.Trace(err)
}
return nil
Expand Down
4 changes: 2 additions & 2 deletions domain/flag/state/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ package state

import (
"context"
"database/sql"

"github.com/juju/errors"
jc "github.com/juju/testing/checkers"
gc "gopkg.in/check.v1"

Expand All @@ -30,7 +30,7 @@ func (s *stateSuite) SetUpTest(c *gc.C) {

func (s *stateSuite) TestGetFlagNotFound(c *gc.C) {
value, err := s.state.GetFlag(context.Background(), "foo")
c.Assert(err, jc.ErrorIs, sql.ErrNoRows)
c.Assert(err, jc.ErrorIs, errors.NotFound)
c.Assert(value, jc.IsFalse)
}

Expand Down
87 changes: 87 additions & 0 deletions internal/bootstrap/agentbinary.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Copyright 2023 Canonical Ltd.
// Licensed under the LGPLv3, see LICENCE file for details.

package bootstrap

import (
"bytes"
"context"
"fmt"
"io"
"os"
"path/filepath"

"github.com/juju/errors"
"github.com/juju/version/v2"

agenttools "github.com/juju/juju/agent/tools"
"github.com/juju/juju/core/arch"
coreos "github.com/juju/juju/core/os"
"github.com/juju/juju/state/binarystorage"
jujuversion "github.com/juju/juju/version"
)

// Logger represents the logging methods called.
type Logger interface {
Errorf(message string, args ...any)
Warningf(message string, args ...any)
Infof(message string, args ...any)
Debugf(message string, args ...any)
}

const (
// AgentCompressedBinaryName is the name of the agent binary.
AgentCompressedBinaryName = "tools.tar.gz"
)

// AgentBinaryStorage is the interface that is used to store the agent binary.
type AgentBinaryStorage interface {
// Add adds the agent binary to the storage.
Add(context.Context, io.Reader, binarystorage.Metadata) error
}

// PopulateAgentBinary is the function that is used to populate the agent
// binary at bootstrap.
func PopulateAgentBinary(ctx context.Context, dataDir string, storage AgentBinaryStorage, logger Logger) error {
current := version.Binary{
Number: jujuversion.Current,
Arch: arch.HostArch(),
Release: coreos.HostOSTypeName(),
}

agentTools, err := agenttools.ReadTools(dataDir, current)
if err != nil {
return fmt.Errorf("cannot read agent binary: %w", err)
}

rootPath := agenttools.SharedToolsDir(dataDir, current)
binaryPath := filepath.Join(rootPath, AgentCompressedBinaryName)

data, err := os.ReadFile(binaryPath)
if err != nil {
return errors.Trace(err)
}

metadata := binarystorage.Metadata{
Version: agentTools.Version.String(),
Size: agentTools.Size,
SHA256: agentTools.SHA256,
}

logger.Debugf("Adding agent binary: %v", agentTools.Version)
if err := storage.Add(ctx, bytes.NewReader(data), metadata); err != nil {
return errors.Trace(err)
}

// Ensure that we remove the agent binary from disk.
if err := os.Remove(binaryPath); err != nil {
logger.Warningf("failed to remove agent binary: %v", err)
}
// Remove the sha that validates the agent binary file.
shaFilePath := filepath.Join(rootPath, fmt.Sprintf("juju%s.sha256", current.String()))
if err := os.Remove(shaFilePath); err != nil {
logger.Warningf("failed to remove agent binary sha: %v", err)
}

return nil
}
Loading

0 comments on commit 8308806

Please sign in to comment.