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

update bootstrap to not recover v2store #16470

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion server/config/v2_deprecation.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const (
// ability to rollback to etcd v3.5.
V2_DEPR_2_GONE = V2DeprecationEnum("gone")

V2_DEPR_DEFAULT = V2_DEPR_1_WRITE_ONLY
V2_DEPR_DEFAULT = V2_DEPR_1_WRITE_ONLY_DROP
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v3.6 should not drop v2 entries as it would break downgrade to v3.5

)

func (e V2DeprecationEnum) IsAtLeast(v2d V2DeprecationEnum) bool {
Expand Down
29 changes: 8 additions & 21 deletions server/etcdserver/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ func bootstrap(cfg config.ServerConfig) (b *bootstrappedServer, err error) {
}

haveWAL := wal.Exist(cfg.WALDir())
st := v2store.New(StoreClusterPrefix, StoreKeysPrefix)
backend, err := bootstrapBackend(cfg, haveWAL, st, ss)
backend, err := bootstrapBackend(cfg, haveWAL, ss)
if err != nil {
return nil, err
}
Expand All @@ -96,7 +95,7 @@ func bootstrap(cfg config.ServerConfig) (b *bootstrappedServer, err error) {
return nil, err
}

s, err := bootstrapStorage(cfg, st, backend, bwal, cluster)
s, err := bootstrapStorage(cfg, backend, bwal, cluster)
if err != nil {
backend.Close()
return nil, err
Expand Down Expand Up @@ -165,14 +164,13 @@ type bootstrappedRaft struct {
storage *raft.MemoryStorage
}

func bootstrapStorage(cfg config.ServerConfig, st v2store.Store, be *bootstrappedBackend, wal *bootstrappedWAL, cl *bootstrapedCluster) (b *bootstrappedStorage, err error) {
func bootstrapStorage(cfg config.ServerConfig, be *bootstrappedBackend, wal *bootstrappedWAL, cl *bootstrapedCluster) (b *bootstrappedStorage, err error) {
if wal == nil {
wal = bootstrapNewWAL(cfg, cl)
}

return &bootstrappedStorage{
backend: be,
st: st,
wal: wal,
}, nil
}
Expand All @@ -198,7 +196,7 @@ func bootstrapSnapshot(cfg config.ServerConfig) *snap.Snapshotter {
return snap.New(cfg.Logger, cfg.SnapDir())
}

func bootstrapBackend(cfg config.ServerConfig, haveWAL bool, st v2store.Store, ss *snap.Snapshotter) (backend *bootstrappedBackend, err error) {
func bootstrapBackend(cfg config.ServerConfig, haveWAL bool, ss *snap.Snapshotter) (backend *bootstrappedBackend, err error) {
beExist := fileutil.Exist(cfg.BackendPath())
ci := cindex.NewConsistentIndex(nil)
beHooks := serverstorage.NewBackendHooks(cfg.Logger, ci)
Expand All @@ -221,7 +219,7 @@ func bootstrapBackend(cfg config.ServerConfig, haveWAL bool, st v2store.Store, s
// TODO(serathius): Implement schema setup in fresh storage
var snapshot *raftpb.Snapshot
if haveWAL {
snapshot, be, err = recoverSnapshot(cfg, st, be, beExist, beHooks, ci, ss)
snapshot, be, err = recoverSnapshot(cfg, be, beExist, beHooks, ci, ss)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -378,7 +376,7 @@ func bootstrapClusterWithWAL(cfg config.ServerConfig, meta *snapshotMetadata) (*
}, nil
}

func recoverSnapshot(cfg config.ServerConfig, st v2store.Store, be backend.Backend, beExist bool, beHooks *serverstorage.BackendHooks, ci cindex.ConsistentIndexer, ss *snap.Snapshotter) (*raftpb.Snapshot, backend.Backend, error) {
func recoverSnapshot(cfg config.ServerConfig, be backend.Backend, beExist bool, beHooks *serverstorage.BackendHooks, ci cindex.ConsistentIndexer, ss *snap.Snapshotter) (*raftpb.Snapshot, backend.Backend, error) {
// Find a snapshot to start/restart a raft node
walSnaps, err := wal.ValidSnapshotEntries(cfg.Logger, cfg.WALDir())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v3.6 should bootstrap from db and not last snapshot

if err != nil {
Expand All @@ -392,21 +390,11 @@ func recoverSnapshot(cfg config.ServerConfig, st v2store.Store, be backend.Backe
}

if snapshot != nil {
if err = st.Recovery(snapshot.Data); err != nil {
cfg.Logger.Panic("failed to recover from snapshot", zap.Error(err))
}

if err = serverstorage.AssertNoV2StoreContent(cfg.Logger, st, cfg.V2Deprecation); err != nil {
cfg.Logger.Error("illegal v2store content", zap.Error(err))
if err = serverstorage.AssertV2DeprecationStage(cfg.Logger, cfg.V2Deprecation); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v3.6 should continue to generate snapshot files to ensure v3.5 backward compatibility.

cfg.Logger.Error("illegal v2store deprecation stage. This version does not recover from v2 snapshot", zap.Error(err))
return nil, be, err
}

cfg.Logger.Info(
"recovered v2 store from snapshot",
zap.Uint64("snapshot-index", snapshot.Metadata.Index),
zap.String("snapshot-size", humanize.Bytes(uint64(snapshot.Size()))),
)

if be, err = serverstorage.RecoverSnapshotBackend(cfg, be, *snapshot, beExist, beHooks); err != nil {
cfg.Logger.Panic("failed to recover v3 backend from snapshot", zap.Error(err))
}
Expand Down Expand Up @@ -446,7 +434,6 @@ func (c *bootstrapedCluster) Finalize(cfg config.ServerConfig, s *bootstrappedSt
if !s.wal.haveWAL {
c.cl.SetID(c.nodeID, c.cl.ID())
}
c.cl.SetStore(s.st)
c.cl.SetBackend(schema.NewMembershipBackend(cfg.Logger, s.backend.be))
if s.wal.haveWAL {
c.cl.Recover(api.UpdateCapability)
Expand Down
15 changes: 5 additions & 10 deletions server/etcdserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func NewServer(cfg config.ServerConfig) (srv *EtcdServer, err error) {
lgMu: new(sync.RWMutex),
lg: cfg.Logger,
errorc: make(chan error, 1),
v2store: b.storage.st,
v2store: v2store.New(StoreClusterPrefix, StoreKeysPrefix),
snapshotter: b.ss,
r: *b.raft.newRaftNode(b.ss, b.storage.wal.w, b.cluster.cl),
memberId: b.cluster.nodeID,
Expand All @@ -340,6 +340,8 @@ func NewServer(cfg config.ServerConfig) (srv *EtcdServer, err error) {
}
serverID.With(prometheus.Labels{"server_id": b.cluster.nodeID.String()}).Set(1)
srv.cluster.SetVersionChangedNotifier(srv.clusterVersionChanged)
//TODO To be removed when RaftCluster does not need the v2store
srv.cluster.SetStore(srv.v2store)
srv.applyV2 = NewApplierV2(cfg.Logger, srv.v2store, srv.cluster)

srv.be = b.storage.backend.be
Expand Down Expand Up @@ -1043,17 +1045,10 @@ func (s *EtcdServer) applySnapshot(ep *etcdProgress, toApply *toApply) {
lg.Info("restored auth store")
}

lg.Info("restoring v2 store")
if err := s.v2store.Recovery(toApply.snapshot.Data); err != nil {
lg.Panic("failed to restore v2 store", zap.Error(err))
if err := serverstorage.AssertV2DeprecationStage(lg, s.Cfg.V2Deprecation); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just not restore v2store when we get it in snapshot. We still expect to get v2store in snapshot when we upgrade v3.5->v3.6 or downgrade.

lg.Panic("illegal v2store deprecation stage. This version does not recover from v2 snapshot", zap.Error(err))
}

if err := serverstorage.AssertNoV2StoreContent(lg, s.v2store, s.Cfg.V2Deprecation); err != nil {
lg.Panic("illegal v2store content", zap.Error(err))
}

lg.Info("restored v2 store")

s.cluster.SetBackend(schema.NewMembershipBackend(lg, newbe))

lg.Info("restoring cluster configuration")
Expand Down
18 changes: 4 additions & 14 deletions server/storage/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,14 @@ import (
"go.etcd.io/etcd/pkg/v3/pbutil"
"go.etcd.io/etcd/server/v3/config"
"go.etcd.io/etcd/server/v3/etcdserver/api/membership"
"go.etcd.io/etcd/server/v3/etcdserver/api/v2store"
"go.etcd.io/raft/v3/raftpb"
)

// AssertNoV2StoreContent -> depending on the deprecation stage, warns or report an error
// if the v2store contains custom content.
func AssertNoV2StoreContent(lg *zap.Logger, st v2store.Store, deprecationStage config.V2DeprecationEnum) error {
metaOnly, err := membership.IsMetaStoreOnly(st)
if err != nil {
return err
func AssertV2DeprecationStage(lg *zap.Logger, deprecationStage config.V2DeprecationEnum) error {
//supported stages are "write-only-drop-data" and "gone"
Copy link
Member

@serathius serathius Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backward compatibility we need to keep write-only

if !deprecationStage.IsAtLeast(config.V2_DEPR_1_WRITE_ONLY_DROP) {
return fmt.Errorf("Unsupported stage --v2-deprecation=%s", deprecationStage)
}
if metaOnly {
return nil
}
if deprecationStage.IsAtLeast(config.V2_DEPR_1_WRITE_ONLY) {
return fmt.Errorf("detected disallowed custom content in v2store for stage --v2-deprecation=%s", deprecationStage)
}
lg.Warn("detected custom v2store content. Etcd v3.5 is the last version allowing to access it using API v2. Please remove the content.")
return nil
}

Expand Down