Skip to content

Commit

Permalink
Store Status of a switch in a separate table (#453)
Browse files Browse the repository at this point in the history
  • Loading branch information
@majst01
majst01 authored Jul 13, 2023
1 parent 517cefa commit c475c1b
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 68 deletions.
7 changes: 6 additions & 1 deletion cmd/metal-api/internal/datastore/migrations_integration/migrate_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ func Test_Migration(t *testing.T) {
CrashLoop: false,
FailedMachineReclaim: false,
// time comparison with time from rethink db is not possible due to different formats
}, cmpopts.IgnoreFields(metal.Base{}, "Changed"), cmpopts.IgnoreFields(metal.ProvisioningEvent{}, "Time"), cmpopts.IgnoreFields(metal.ProvisioningEventContainer{}, "LastEventTime")); diff != "" {
},
cmpopts.IgnoreFields(metal.Base{}, "Changed"),
cmpopts.IgnoreFields(metal.ProvisioningEvent{}, "Time"),
cmpopts.IgnoreFields(metal.ProvisioningEventContainer{}, "LastEventTime"),
cmpopts.IgnoreFields(metal.ProvisioningEventContainer{}, "Created"),
); diff != "" {
t.Errorf("RethinkStore.Migrate() mismatch (-want +got):\n%s", diff)
}

Expand Down
11 changes: 7 additions & 4 deletions cmd/metal-api/internal/datastore/rethinkdb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const (
)

var tables = []string{
"image", "size", "partition", "machine", "switch", "event", "network", "ip", "migration", "filesystemlayout", "sizeimageconstraint",
"image", "size", "partition", "machine", "switch", "switchstatus", "event", "network", "ip", "migration", "filesystemlayout", "sizeimageconstraint",
VRFIntegerPool.String(), VRFIntegerPool.String() + "info",
ASNIntegerPool.String(), ASNIntegerPool.String() + "info",
}
Expand Down Expand Up @@ -161,7 +161,10 @@ func (rs *RethinkStore) switchTable() *r.Term {
res := r.DB(rs.dbname).Table("switch")
return &res
}

func (rs *RethinkStore) switchStatusTable() *r.Term {
res := r.DB(rs.dbname).Table("switchstatus")
return &res
}
func (rs *RethinkStore) eventTable() *r.Term {
res := r.DB(rs.dbname).Table("event")
return &res
Expand Down Expand Up @@ -394,8 +397,8 @@ func (rs *RethinkStore) createEntity(table *r.Term, entity metal.Entity) error {

func (rs *RethinkStore) upsertEntity(table *r.Term, entity metal.Entity) error {
now := time.Now()
if entity.GetChanged().IsZero() {
entity.SetChanged(now)
if entity.GetCreated().IsZero() {
entity.SetCreated(now)
}
entity.SetChanged(now)

Expand Down
15 changes: 15 additions & 0 deletions cmd/metal-api/internal/datastore/switch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,18 @@ func (rs *RethinkStore) ConnectMachineWithSwitches(m *metal.Machine) error {

return nil
}

// GetSwitchStatus get SwitchStatus for a given switch id
func (rs *RethinkStore) GetSwitchStatus(id string) (*metal.SwitchStatus, error) {
var ss metal.SwitchStatus
err := rs.findEntityByID(rs.switchStatusTable(), &ss, id)
if err != nil && !metal.IsNotFound(err) {
return nil, err
}
return &ss, nil
}

// SetSwitchStatus create or update the switch status.
func (rs *RethinkStore) SetSwitchStatus(state *metal.SwitchStatus) error {
return rs.upsertEntity(rs.switchStatusTable(), state)
}
9 changes: 7 additions & 2 deletions cmd/metal-api/internal/metal/switch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ type Switch struct {
PartitionID string `rethinkdb:"partitionid" json:"partitionid"`
RackID string `rethinkdb:"rackid" json:"rackid"`
Mode SwitchMode `rethinkdb:"mode" json:"mode"`
LastSync *SwitchSync `rethinkdb:"last_sync" json:"last_sync"`
LastSyncError *SwitchSync `rethinkdb:"last_sync_error" json:"last_sync_error"`
OS *SwitchOS `rethinkdb:"os" json:"os"`
ManagementIP string `rethinkdb:"management_ip" json:"management_ip"`
ManagementUser string `rethinkdb:"management_user" json:"management_user"`
Expand Down Expand Up @@ -59,6 +57,13 @@ type SwitchEvent struct {
Switches []Switch `json:"switches"`
}

// SwitchStatus stores the received switch notifications in a separate table
type SwitchStatus struct {
Base
LastSync *SwitchSync `rethinkdb:"last_sync" json:"last_sync" description:"last successful synchronization to the switch" optional:"true"`
LastSyncError *SwitchSync `rethinkdb:"last_sync_error" json:"last_sync_error" description:"last synchronization to the switch that was erroneous" optional:"true"`
}

// SwitchSync contains information about the last synchronization of the state held in the metal-api to a switch.
type SwitchSync struct {
Time time.Time `rethinkdb:"time" json:"time"`
Expand Down
56 changes: 30 additions & 26 deletions cmd/metal-api/internal/service/switch-service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (r *switchResource) webService() *restful.WebService {
Metadata(restfulspec.KeyOpenAPITags, tags).
Metadata(auditing.Exclude, true).
Reads(v1.SwitchNotifyRequest{}).
Returns(http.StatusOK, "OK", v1.SwitchResponse{}).
Returns(http.StatusOK, "OK", v1.SwitchNotifyResponse{}).
DefaultReturns("Error", httperrors.HTTPErrorResponse{}))

return ws
Expand Down Expand Up @@ -210,38 +210,33 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu
}

id := request.PathParameter("id")
s, err := r.ds.FindSwitch(id)
if err != nil && !metal.IsNotFound(err) {
r.sendError(request, response, defaultError(err))
return
}

old := *s
sync := &metal.SwitchSync{
Time: time.Now(),
Duration: requestPayload.Duration,
}
ss := &metal.SwitchStatus{
Base: metal.Base{ID: id},
}

resp := &v1.SwitchNotifyResponse{
Common: v1.Common{Identifiable: v1.Identifiable{ID: id}},
}

if requestPayload.Error == nil {
s.LastSync = sync
resp.LastSync = &v1.SwitchSync{Time: sync.Time, Duration: sync.Duration}
ss.LastSync = sync
} else {
resp.LastSyncError = &v1.SwitchSync{Time: sync.Time, Duration: sync.Duration, Error: requestPayload.Error}
sync.Error = requestPayload.Error
s.LastSyncError = sync
}

// FIXME needs https://github.com/metal-stack/metal-api/issues/263
err = r.ds.UpdateSwitch(&old, s)
if err != nil {
r.sendError(request, response, defaultError(err))
return
ss.LastSyncError = sync
}

resp, err := makeSwitchResponse(s, r.ds)
err = r.ds.SetSwitchStatus(ss)
if err != nil {
r.sendError(request, response, defaultError(err))
return
}

r.send(request, response, http.StatusOK, resp)
}

Expand Down Expand Up @@ -621,15 +616,15 @@ func updateSwitchNics(oldNics, newNics map[string]*metal.Nic, currentConnections
}

func makeSwitchResponse(s *metal.Switch, ds *datastore.RethinkStore) (*v1.SwitchResponse, error) {
p, ips, machines, err := findSwitchReferencedEntites(s, ds)
p, ips, machines, ss, err := findSwitchReferencedEntites(s, ds)
if err != nil {
return nil, err
}

nics := makeSwitchNics(s, ips, machines)
cons := makeSwitchCons(s)

return v1.NewSwitchResponse(s, p, nics, cons), nil
return v1.NewSwitchResponse(s, ss, p, nics, cons), nil
}

func makeBGPFilterFirewall(m metal.Machine) v1.BGPFilter {
Expand Down Expand Up @@ -767,29 +762,34 @@ func makeSwitchCons(s *metal.Switch) []v1.SwitchConnection {
return cons
}

func findSwitchReferencedEntites(s *metal.Switch, ds *datastore.RethinkStore) (*metal.Partition, metal.IPsMap, metal.Machines, error) {
func findSwitchReferencedEntites(s *metal.Switch, ds *datastore.RethinkStore) (*metal.Partition, metal.IPsMap, metal.Machines, *metal.SwitchStatus, error) {
var err error
var p *metal.Partition
var m metal.Machines

if s.PartitionID != "" {
p, err = ds.FindPartition(s.PartitionID)
if err != nil {
return nil, nil, nil, fmt.Errorf("switch %q references partition, but partition %q cannot be found in database: %w", s.ID, s.PartitionID, err)
return nil, nil, nil, nil, fmt.Errorf("switch %q references partition, but partition %q cannot be found in database: %w", s.ID, s.PartitionID, err)
}

err = ds.SearchMachines(&datastore.MachineSearchQuery{PartitionID: &s.PartitionID}, &m)
if err != nil {
return nil, nil, nil, fmt.Errorf("could not search machines of partition %q for switch %q: %w", s.PartitionID, s.ID, err)
return nil, nil, nil, nil, fmt.Errorf("could not search machines of partition %q for switch %q: %w", s.PartitionID, s.ID, err)
}
}

ips, err := ds.ListIPs()
if err != nil {
return nil, nil, nil, fmt.Errorf("ips could not be listed: %w", err)
return nil, nil, nil, nil, fmt.Errorf("ips could not be listed: %w", err)
}

return p, ips.ByProjectID(), m, nil
ss, err := ds.GetSwitchStatus(s.ID)
if err != nil && !metal.IsNotFound(err) {
return nil, nil, nil, nil, fmt.Errorf("switchStatus could not be listed: %w", err)
}

return p, ips.ByProjectID(), m, ss, nil
}

func makeSwitchResponseList(ss metal.Switches, ds *datastore.RethinkStore) ([]*v1.SwitchResponse, error) {
Expand All @@ -814,7 +814,11 @@ func makeSwitchResponseList(ss metal.Switches, ds *datastore.RethinkStore) ([]*v

nics := makeSwitchNics(&sw, ips, m)
cons := makeSwitchCons(&sw)
result = append(result, v1.NewSwitchResponse(&sw, p, nics, cons))
ss, err := ds.GetSwitchStatus(sw.ID)
if err != nil && !metal.IsNotFound(err) {
return nil, err
}
result = append(result, v1.NewSwitchResponse(&sw, ss, p, nics, cons))
}

return result, nil
Expand Down
41 changes: 21 additions & 20 deletions cmd/metal-api/internal/service/switch-service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1020,10 +1020,10 @@ func Test_updateSwitchNics(t *testing.T) {
name: "idempotence",
args: args{
oldNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
},
newNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
},
currentConnections: metal.ConnectionMap{},
},
Expand All @@ -1036,11 +1036,11 @@ func Test_updateSwitchNics(t *testing.T) {
name: "adding a nic",
args: args{
oldNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
},
newNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:12": &metal.Nic{Name: "swp2", MacAddress: "11:11:11:11:11:12"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:12": {Name: "swp2", MacAddress: "11:11:11:11:11:12"},
},
currentConnections: metal.ConnectionMap{},
},
Expand All @@ -1054,7 +1054,7 @@ func Test_updateSwitchNics(t *testing.T) {
name: "removing a nic",
args: args{
oldNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
},
newNics: map[string]*metal.Nic{},
currentConnections: metal.ConnectionMap{},
Expand All @@ -1066,11 +1066,11 @@ func Test_updateSwitchNics(t *testing.T) {
name: "removing a nic 2",
args: args{
oldNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:12": &metal.Nic{Name: "swp2", MacAddress: "11:11:11:11:11:12"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:12": {Name: "swp2", MacAddress: "11:11:11:11:11:12"},
},
newNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
},
currentConnections: metal.ConnectionMap{},
},
Expand All @@ -1083,11 +1083,11 @@ func Test_updateSwitchNics(t *testing.T) {
name: "removing a used nic",
args: args{
oldNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:12": &metal.Nic{Name: "swp2", MacAddress: "11:11:11:11:11:12"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:12": {Name: "swp2", MacAddress: "11:11:11:11:11:12"},
},
newNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
},
currentConnections: metal.ConnectionMap{
"machine-uuid-1": metal.Connections{metal.Connection{MachineID: "machine-uuid-1", Nic: metal.Nic{Name: "swp2", MacAddress: "11:11:11:11:11:12"}}},
Expand All @@ -1100,10 +1100,10 @@ func Test_updateSwitchNics(t *testing.T) {
name: "updating a nic",
args: args{
oldNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
},
newNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp2", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp2", MacAddress: "11:11:11:11:11:11"},
},
currentConnections: metal.ConnectionMap{},
},
Expand All @@ -1116,10 +1116,10 @@ func Test_updateSwitchNics(t *testing.T) {
name: "updating a nic, vrf should not be altered",
args: args{
oldNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", Vrf: "vrf1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp1", Vrf: "vrf1", MacAddress: "11:11:11:11:11:11"},
},
newNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp2", Vrf: "vrf2", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp2", Vrf: "vrf2", MacAddress: "11:11:11:11:11:11"},
},
currentConnections: metal.ConnectionMap{},
},
Expand All @@ -1132,10 +1132,10 @@ func Test_updateSwitchNics(t *testing.T) {
name: "updating a nic name, which already has a connection",
args: args{
oldNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp1", MacAddress: "11:11:11:11:11:11"},
},
newNics: map[string]*metal.Nic{
"11:11:11:11:11:11": &metal.Nic{Name: "swp2", MacAddress: "11:11:11:11:11:11"},
"11:11:11:11:11:11": {Name: "swp2", MacAddress: "11:11:11:11:11:11"},
},
currentConnections: metal.ConnectionMap{
"machine-uuid-1": metal.Connections{metal.Connection{MachineID: "machine-uuid-1", Nic: metal.Nic{Name: "swp1", MacAddress: "11:11:11:11:11:11"}}},
Expand Down Expand Up @@ -1229,11 +1229,12 @@ func TestNotifySwitch(t *testing.T) {
resp := w.Result()
defer resp.Body.Close()
require.Equal(t, http.StatusOK, resp.StatusCode, w.Body.String())
var result v1.SwitchResponse
var result v1.SwitchNotifyResponse
err = json.NewDecoder(resp.Body).Decode(&result)

require.NoError(t, err)
require.Equal(t, id, result.ID)
require.NotNil(t, result.LastSync)
require.Equal(t, d, result.LastSync.Duration)
require.Nil(t, result.LastSyncError)
}
Expand Down Expand Up @@ -1265,7 +1266,7 @@ func TestNotifyErrorSwitch(t *testing.T) {
resp := w.Result()
defer resp.Body.Close()
require.Equal(t, http.StatusOK, resp.StatusCode, w.Body.String())
var result v1.SwitchResponse
var result v1.SwitchNotifyResponse
err = json.NewDecoder(resp.Body).Decode(&result)

require.NoError(t, err)
Expand Down
Loading

0 comments on commit c475c1b

Please sign in to comment.