Skip to content
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

[JUJU-1950] Use the new lease store in the lease manager #15002

Merged
merged 21 commits into from
Dec 16, 2022
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
676123a
Wraps lease store constructor arguments in a single config struct.
manadart Dec 8, 2022
49ee52c
Ensures that foreign keys are enforced when testing the lease store with
manadart Dec 12, 2022
a71944e
Removes globalclockupdater worker. This is not required by the DB-backed
manadart Dec 12, 2022
d845279
Removes all raft-based concerns from the apiserver worker.
manadart Dec 12, 2022
7505cc8
Removes all raft-based concerms from the API server.
manadart Dec 12, 2022
afa21e0
Removes all raft workers. They are no longer required since we are
manadart Dec 12, 2022
67d9184
Removes Raft dependencies from httpserver worker.
manadart Dec 12, 2022
2290879
Modifies agent bootstrap so that it no longer initialises Raft.
manadart Dec 12, 2022
cebd81d
Removes the now-unused Raft queue logic.
manadart Dec 12, 2022
e4b0ee9
Changes lease manager manifold to work with the new DB-backed lease
manadart Dec 12, 2022
c578384
Removes unrequired hub from lease manager manifold config and fixes
manadart Dec 13, 2022
35e3671
Updates lease type names to conform with those in use throughout Juju.
manadart Dec 13, 2022
50b5eec
Removes the Raft operation queue creation from the machine agent. It is
manadart Dec 13, 2022
c8eca2f
Threads the db-accessor into the lease manager worker, and uses it to
manadart Dec 13, 2022
40bce47
Creates a new simple test suite for testing controller concerns with
manadart Dec 15, 2022
96fef13
Adds a new worker that constantly deletes expired leases from the
manadart Dec 15, 2022
04764af
Adds the lease expiry worker to the common machine manifolds config so
manadart Dec 15, 2022
43242ff
Removes lease error handling from the API server where it was instituted
manadart Dec 16, 2022
6b2e8f0
Ensures that the lease manager retries errors due to database
manadart Dec 16, 2022
82d7fad
Adds a fallback for unique constraint violation errors by interrogating
manadart Dec 16, 2022
e0fe075
Indicates DB lock errors as a timeout rather than crashing the lease
manadart Dec 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions agent/agentbootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ package agentbootstrap
import (
stdcontext "context"
"fmt"
"path/filepath"

coreraft "github.com/hashicorp/raft"
"github.com/juju/clock"
"github.com/juju/errors"
"github.com/juju/loggo"
Expand All @@ -27,7 +25,6 @@ import (
"github.com/juju/juju/core/instance"
"github.com/juju/juju/core/model"
corenetwork "github.com/juju/juju/core/network"
"github.com/juju/juju/core/raft/queue"
"github.com/juju/juju/core/series"
"github.com/juju/juju/database"
"github.com/juju/juju/environs"
Expand All @@ -39,7 +36,6 @@ import (
"github.com/juju/juju/network"
"github.com/juju/juju/state"
"github.com/juju/juju/storage"
"github.com/juju/juju/worker/raft"
)

var logger = loggo.GetLogger("juju.agent.agentbootstrap")
Expand Down Expand Up @@ -110,10 +106,6 @@ func InitializeState(
info.Tag = nil
info.Password = c.OldPassword()

if err := initRaft(c); err != nil {
return nil, errors.Trace(err)
}

if err := database.BootstrapDqlite(stdcontext.TODO(), database.NewOptionFactory(c, logger), logger); err != nil {
return nil, errors.Trace(err)
}
Expand Down Expand Up @@ -406,17 +398,6 @@ func getEnviron(
return environs.Open(stdcontext.TODO(), provider, openParams)
}

func initRaft(agentConfig agent.Config) error {
raftDir := filepath.Join(agentConfig.DataDir(), "raft")
return raft.Bootstrap(raft.Config{
Clock: clock.WallClock,
StorageDir: raftDir,
Logger: logger,
LocalID: coreraft.ServerID(agentConfig.Tag().Id()),
Queue: queue.NewOpQueue(clock.WallClock),
})
}

// initMongo dials the initial MongoDB connection, setting a
// password for the admin user, and returning the session.
func initMongo(info mongo.Info, dialOpts mongo.DialOpts, password string) (*mgo.Session, error) {
Expand Down
2 changes: 0 additions & 2 deletions apiserver/allfacades.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ import (
"github.com/juju/juju/apiserver/facades/controller/metricsmanager"
"github.com/juju/juju/apiserver/facades/controller/migrationmaster"
"github.com/juju/juju/apiserver/facades/controller/migrationtarget"
"github.com/juju/juju/apiserver/facades/controller/raftlease"
"github.com/juju/juju/apiserver/facades/controller/remoterelations"
"github.com/juju/juju/apiserver/facades/controller/singular"
"github.com/juju/juju/apiserver/facades/controller/statushistory"
Expand Down Expand Up @@ -187,7 +186,6 @@ func AllFacades() *facade.Registry {
payloadshookcontext.Register(registry)
provisioner.Register(registry)
proxyupdater.Register(registry)
raftlease.Register(registry)
reboot.Register(registry)
remoterelations.Register(registry)
resources.Register(registry)
Expand Down
8 changes: 0 additions & 8 deletions apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,6 @@ type ServerConfig struct {
// ExecEmbeddedCommand is a function which creates an embedded Juju CLI instance.
ExecEmbeddedCommand ExecEmbeddedCommandFunc

// RaftOpQueue is used by the API to apply operations to the raft
// instance.
RaftOpQueue Queue

// CharmhubHTTPClient is the HTTP client used for Charmhub API requests.
CharmhubHTTPClient facade.HTTPClient
}
Expand Down Expand Up @@ -253,9 +249,6 @@ func (c ServerConfig) Validate() error {
if c.MetricsCollector == nil {
return errors.NotValidf("missing MetricsCollector")
}
if c.RaftOpQueue == nil {
return errors.NotValidf("missing RaftOpQueue")
}
return nil
}

Expand Down Expand Up @@ -303,7 +296,6 @@ func newServer(cfg ServerConfig) (_ *Server, err error) {
presence: cfg.Presence,
leaseManager: cfg.LeaseManager,
controllerConfig: controllerConfig,
raftOpQueue: cfg.RaftOpQueue,
logger: loggo.GetLogger("juju.apiserver"),
charmhubHTTPClient: cfg.CharmhubHTTPClient,
})
Expand Down
5 changes: 1 addition & 4 deletions apiserver/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (

"github.com/gorilla/websocket"
"github.com/juju/clock"
"github.com/juju/clock/testclock"
"github.com/juju/cmd/v3"
"github.com/juju/collections/set"
jujuhttp "github.com/juju/http/v2"
Expand All @@ -40,7 +39,6 @@ import (
"github.com/juju/juju/core/cache"
corelogger "github.com/juju/juju/core/logger"
"github.com/juju/juju/core/presence"
"github.com/juju/juju/core/raft/queue"
"github.com/juju/juju/jujuclient"
psapiserver "github.com/juju/juju/pubsub/apiserver"
"github.com/juju/juju/pubsub/centralhub"
Expand Down Expand Up @@ -180,8 +178,7 @@ func (s *apiserverConfigFixture) SetUpTest(c *gc.C) {
}
return 0
},
SysLogger: noopSysLogger{},
RaftOpQueue: queue.NewOpQueue(testclock.NewClock(time.Now())),
SysLogger: noopSysLogger{},
}
}

Expand Down
7 changes: 0 additions & 7 deletions apiserver/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,6 @@ func ServerErrorAndStatus(err error) (*params.Error, int) {
status = http.StatusConflict
case params.CodeNotLeader:
status = http.StatusTemporaryRedirect
case params.CodeLeaseError:
status = leaseStatusCode(err1)
}
return err1, status
}
Expand Down Expand Up @@ -252,9 +250,6 @@ func ServerError(err error) *params.Error {
info = notLeaderError.AsMap()
case errors.Is(err, DeadlineExceededError):
code = params.CodeDeadlineExceeded
case lease.IsLeaseError(err):
code = params.CodeLeaseError
info = leaseErrorInfoMap(err)
default:
code = params.ErrCode(err)
}
Expand Down Expand Up @@ -347,8 +342,6 @@ func RestoreError(err error) error {
return NewNotLeaderError(serverAddress, serverID)
case params.IsCodeDeadlineExceeded(err):
return fmt.Errorf(msg+"%w", errors.Hide(DeadlineExceededError))
case params.IsLeaseError(err):
return rehydrateLeaseError(err)
case params.IsCodeTryAgain(err):
return ErrTryAgain
default:
Expand Down
39 changes: 0 additions & 39 deletions apiserver/errors/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,51 +269,12 @@ var errorTransformTests = []struct {
err: apiservererrors.DeadlineExceededError,
code: params.CodeDeadlineExceeded,
status: http.StatusInternalServerError,
}, {
err: lease.ErrHeld,
code: params.CodeLeaseError,
status: http.StatusBadRequest,
helperFunc: leaseErrHelperFunc(lease.ErrHeld),
}, {
err: lease.ErrInvalid,
code: params.CodeLeaseError,
status: http.StatusBadRequest,
helperFunc: leaseErrHelperFunc(lease.ErrInvalid),
}, {
err: lease.ErrTimeout,
code: params.CodeLeaseError,
status: http.StatusInternalServerError,
helperFunc: leaseErrHelperFunc(lease.ErrTimeout),
}, {
err: lease.ErrNotHeld,
code: params.CodeLeaseError,
status: http.StatusBadRequest,
helperFunc: leaseErrHelperFunc(lease.ErrNotHeld),
}, {
err: lease.ErrDropped,
code: params.CodeLeaseError,
status: http.StatusInternalServerError,
helperFunc: leaseErrHelperFunc(lease.ErrDropped),
}, {
err: nil,
code: "",
status: http.StatusOK,
}}

func leaseErrHelperFunc(leaseErr error) func(error) bool {
return func(err error) bool {
err1, ok := err.(*params.Error)
if !ok {
return false
}
m := apiservererrors.LeaseErrorInfoMap(leaseErr)
if err1.Info == nil || !reflect.DeepEqual(err1.Info, m) {
return false
}
return true
}
}

var sampleMacaroon = func() *macaroon.Macaroon {
m, err := macaroon.New([]byte("key"), []byte("id"), "loc", macaroon.LatestVersion)
if err != nil {
Expand Down
8 changes: 0 additions & 8 deletions apiserver/errors/export_test.go

This file was deleted.

75 changes: 0 additions & 75 deletions apiserver/errors/lease.go

This file was deleted.

7 changes: 1 addition & 6 deletions apiserver/facade/facadetest/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ type Context struct {
LeadershipPinner_ leadership.Pinner
LeadershipReader_ leadership.Reader
SingularClaimer_ lease.Claimer
Raft_ facade.RaftContext
CharmhubHTTPClient_ facade.HTTPClient
// Identity is not part of the facade.Context interface, but is instead
// used to make sure that the context objects are the same.
Expand Down Expand Up @@ -132,7 +131,7 @@ func (context Context) LeadershipPinner(modelUUID string) (leadership.Pinner, er
return context.LeadershipPinner_, nil
}

// LeadershipPinner implements facade.Context.
// LeadershipReader implements facade.Context.
func (context Context) LeadershipReader(modelUUID string) (leadership.Reader, error) {
return context.LeadershipReader_, nil
}
Expand All @@ -142,10 +141,6 @@ func (context Context) SingularClaimer() (lease.Claimer, error) {
return context.SingularClaimer_, nil
}

func (context Context) Raft() facade.RaftContext {
return context.Raft_
}

func (context Context) HTTPClient(purpose facade.HTTPClientPurpose) facade.HTTPClient {
switch purpose {
case facade.CharmhubHTTPClient:
Expand Down
17 changes: 0 additions & 17 deletions apiserver/facade/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package facade

import (
"context"
"net/http"
"net/url"
"time"
Expand All @@ -17,7 +16,6 @@ import (
"github.com/juju/juju/core/multiwatcher"
"github.com/juju/juju/core/permission"
"github.com/juju/juju/core/presence"
"github.com/juju/juju/core/raftlease"
"github.com/juju/juju/state"
)

Expand Down Expand Up @@ -58,18 +56,6 @@ type LeadershipContext interface {
SingularClaimer() (lease.Claimer, error)
}

// RaftContext describes methods for handling raft related capabilities.
type RaftContext interface {

// ApplyLease attempts to apply the command on to the raft FSM. It only
// takes a command and enqueues that against the raft instance. If the raft
// instance is already processing a application, then back pressure is
// applied to the caller and a ErrEnqueueDeadlineExceeded will be sent.
// It's up to the caller to retry or drop depending on how the retry
// algorithm is implemented.
ApplyLease(context.Context, raftlease.Command) error
}

// Context exposes useful capabilities to a Facade.
type Context interface {
// TODO (stickupkid): This shouldn't be embedded, instead this should be
Expand Down Expand Up @@ -158,9 +144,6 @@ type Context interface {
// RequestRecorder defines a metrics collector for outbound requests.
RequestRecorder() RequestRecorder

// Raft returns a lease context for managing raft.
Raft() RaftContext

// HTTPClient returns an HTTP client to use for the given purpose.
HTTPClient(purpose HTTPClientPurpose) HTTPClient
}
Expand Down
Loading