Skip to content

Commit 52e6834

Browse files
author
Mikhail Podtserkovskiy
committed
QA-7710: refactoring
1 parent 9962a10 commit 52e6834

File tree

12 files changed

+69
-41
lines changed

12 files changed

+69
-41
lines changed

‎handlers/registerNode.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,11 @@ func (h *RegisterNode) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
5050
for i, value := range capabilitiesList {
5151
poolCapabilitiesList[i] = capabilities.Capabilities(value)
5252
}
53+
hostPort := register.Configuration.Host + ":" + strconv.Itoa(register.Configuration.Port)
5354
err = h.Pool.Add(
55+
hostPort,
5456
pool.NodeTypePersistent,
55-
register.Configuration.Host+":"+strconv.Itoa(register.Configuration.Port),
57+
hostPort,
5658
poolCapabilitiesList,
5759
)
5860
if err != nil {

‎pool/node.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const (
1818
)
1919

2020
type Node struct {
21+
Key string
2122
Type NodeType
2223
Address string
2324
Status NodeStatus
@@ -32,6 +33,7 @@ func (n *Node) String() string {
3233
}
3334

3435
func NewNode(
36+
key string,
3537
t NodeType,
3638
address string,
3739
status NodeStatus,
@@ -41,6 +43,7 @@ func NewNode(
4143
capabilitiesList []capabilities.Capabilities,
4244
) *Node {
4345
return &Node{
46+
key,
4447
t,
4548
address,
4649
status,

‎pool/pool.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,12 @@ func (p *Pool) ReserveAvailableNode(caps capabilities.Capabilities) (*Node, erro
6868
return &node, err
6969
}
7070

71-
func (p *Pool) Add(t NodeType, address string, capabilitiesList []capabilities.Capabilities) error {
71+
func (p *Pool) Add(key string, t NodeType, address string, capabilitiesList []capabilities.Capabilities) error {
7272
if len(capabilitiesList) == 0 {
7373
return errors.New("[Pool/Add] Capabilities must contains more one element")
7474
}
7575
ts := time.Now().Unix()
76-
return p.storage.Add(*NewNode(t, address, NodeStatusAvailable, "", ts, ts, capabilitiesList), 0)
76+
return p.storage.Add(*NewNode(key, t, address, NodeStatusAvailable, "", ts, ts, capabilitiesList), 0)
7777
}
7878

7979
func (p *Pool) RegisterSession(node *Node, sessionID string) error {

‎pool/pool_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func TestPool_Add_Positive(t *testing.T) {
4747
p := NewPool(s, new(StrategyListMock))
4848
eAddress := "127.0.0.1"
4949
eNodeType := NodeTypePersistent
50-
err := p.Add(eNodeType, eAddress, []capabilities.Capabilities{{"browserName": "ololo"}})
50+
err := p.Add("1234", eNodeType, eAddress, []capabilities.Capabilities{{"browserName": "ololo"}})
5151
a.Nil(err)
5252
}
5353

@@ -59,7 +59,7 @@ func TestPool_Add_Negative(t *testing.T) {
5959
p := NewPool(s, new(StrategyListMock))
6060
eAddress := "127.0.0.1"
6161
eNodeType := NodeTypePersistent
62-
err := p.Add(eNodeType, eAddress, []capabilities.Capabilities{})
62+
err := p.Add("1234", eNodeType, eAddress, []capabilities.Capabilities{})
6363
a.Error(err)
6464
}
6565

@@ -225,7 +225,7 @@ func TestPool_fixNodeStatus_Positive_BusyExpired(t *testing.T) {
225225
slm := new(StrategyListMock)
226226
slm.On("FixNodeStatus", mock.AnythingOfType("pool.Node")).Return(nil)
227227
p := NewPool(new(StorageMock), slm)
228-
node := NewNode(NodeTypePersistent, "", NodeStatusBusy, "", 0, 0, []capabilities.Capabilities{})
228+
node := NewNode("123", NodeTypePersistent, "", NodeStatusBusy, "", 0, 0, []capabilities.Capabilities{})
229229
isFixed, err := p.fixNodeStatus(node)
230230
a.True(isFixed)
231231
a.Nil(err)
@@ -236,7 +236,7 @@ func TestPool_fixNodeStatus_Positive_ReservedExpired(t *testing.T) {
236236
slm := new(StrategyListMock)
237237
slm.On("FixNodeStatus", mock.AnythingOfType("pool.Node")).Return(nil)
238238
p := NewPool(new(StorageMock), slm)
239-
node := NewNode(NodeTypePersistent, "", NodeStatusReserved, "", 0, 0, []capabilities.Capabilities{})
239+
node := NewNode("123", NodeTypePersistent, "", NodeStatusReserved, "", 0, 0, []capabilities.Capabilities{})
240240
isFixed, err := p.fixNodeStatus(node)
241241
a.True(isFixed)
242242
a.Nil(err)
@@ -247,7 +247,7 @@ func TestPool_fixNodeStatus_Positive_BusyNotNotExpired(t *testing.T) {
247247
slm := new(StrategyListMock)
248248
slm.On("FixNodeStatus", mock.AnythingOfType("pool.Node")).Return(nil)
249249
p := NewPool(new(StorageMock), slm)
250-
node := NewNode(NodeTypePersistent, "", NodeStatusBusy, "", time.Now().Unix(), 0, []capabilities.Capabilities{})
250+
node := NewNode("123", NodeTypePersistent, "", NodeStatusBusy, "", time.Now().Unix(), 0, []capabilities.Capabilities{})
251251
isFixed, err := p.fixNodeStatus(node)
252252
a.False(isFixed)
253253
a.Nil(err)
@@ -258,7 +258,7 @@ func TestPool_fixNodeStatus_Positive_ReservedNotNotExpired(t *testing.T) {
258258
slm := new(StrategyListMock)
259259
slm.On("FixNodeStatus", mock.AnythingOfType("pool.Node")).Return(nil)
260260
p := NewPool(new(StorageMock), slm)
261-
node := NewNode(NodeTypePersistent, "", NodeStatusReserved, "", time.Now().Unix(), 0, []capabilities.Capabilities{})
261+
node := NewNode("123", NodeTypePersistent, "", NodeStatusReserved, "", time.Now().Unix(), 0, []capabilities.Capabilities{})
262262
isFixed, err := p.fixNodeStatus(node)
263263
a.False(isFixed)
264264
a.Nil(err)
@@ -269,7 +269,7 @@ func TestPool_fixNodeStatus_Positive_AvailableExpired(t *testing.T) {
269269
slm := new(StrategyListMock)
270270
slm.On("FixNodeStatus", mock.AnythingOfType("pool.Node")).Return(nil)
271271
p := NewPool(new(StorageMock), slm)
272-
node := NewNode(NodeTypePersistent, "", NodeStatusAvailable, "", 0, 0, []capabilities.Capabilities{})
272+
node := NewNode("123", NodeTypePersistent, "", NodeStatusAvailable, "", 0, 0, []capabilities.Capabilities{})
273273
isFixed, err := p.fixNodeStatus(node)
274274
a.False(isFixed)
275275
a.Nil(err)
@@ -281,7 +281,7 @@ func TestPool_fixNodeStatus_NegativeBusy(t *testing.T) {
281281
slm := new(StrategyListMock)
282282
slm.On("FixNodeStatus", mock.AnythingOfType("pool.Node")).Return(eError)
283283
p := NewPool(new(StorageMock), slm)
284-
node := NewNode(NodeTypePersistent, "", NodeStatusBusy, "", 0, 0, []capabilities.Capabilities{})
284+
node := NewNode("123", NodeTypePersistent, "", NodeStatusBusy, "", 0, 0, []capabilities.Capabilities{})
285285
isFixed, err := p.fixNodeStatus(node)
286286
a.False(isFixed)
287287
a.Error(err)
@@ -293,7 +293,7 @@ func TestPool_fixNodeStatus_NegativeReserved(t *testing.T) {
293293
slm := new(StrategyListMock)
294294
slm.On("FixNodeStatus", mock.AnythingOfType("pool.Node")).Return(eError)
295295
p := NewPool(new(StorageMock), slm)
296-
node := NewNode(NodeTypePersistent, "", NodeStatusReserved, "", 0, 0, []capabilities.Capabilities{})
296+
node := NewNode("123", NodeTypePersistent, "", NodeStatusReserved, "", 0, 0, []capabilities.Capabilities{})
297297
isFixed, err := p.fixNodeStatus(node)
298298
a.False(isFixed)
299299
a.Error(err)

‎pool/strategy/kubernetes/factory.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (f *StrategyFactory) Create(
4242
return nil, errors.New("create k8s clientset, " + err.Error())
4343
}
4444

45-
provider := &kubernetesProvider{
45+
provider := &kubDnsProvider{
4646
clientset: clientset,
4747
namespace: "default", //todo: брать из конфига !!!
4848
clientFactory: clientFactory,

‎pool/strategy/kubernetes/provider.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,22 @@ import (
99
"net"
1010
"strconv"
1111
"time"
12+
"strings"
1213
)
1314

1415
type kubernetesProviderInterface interface {
1516
Create(podName string, nodeParams nodeParams) error
17+
// idempotent operation
1618
Destroy(podName string) error
1719
}
1820

19-
type kubernetesProvider struct {
21+
type kubDnsProvider struct {
2022
clientset *kubernetes.Clientset
2123
namespace string
2224
clientFactory jsonwire.ClientFactoryInterface
2325
}
2426

25-
func (p *kubernetesProvider) Create(podName string, nodeParams nodeParams) error {
27+
func (p *kubDnsProvider) Create(podName string, nodeParams nodeParams) error {
2628
pod := &apiV1.Pod{}
2729
pod.ObjectMeta.Name = podName
2830
pod.ObjectMeta.Labels = map[string]string{"name": podName}
@@ -78,16 +80,22 @@ Loop:
7880
return nil
7981
}
8082

81-
func (p *kubernetesProvider) Destroy(podName string) error {
83+
//Destroy - destroy all pod data (idempotent operation)
84+
func (p *kubDnsProvider) Destroy(podName string) error {
8285
err := p.clientset.CoreV1Client.Pods(p.namespace).Delete(podName, &apiV1.DeleteOptions{})
83-
if err != nil {
86+
switch {
87+
case err != nil && strings.Contains(err.Error(), "not found"):
88+
// pod already deleted
89+
case err != nil:
8490
err = errors.New("send command pod/delete to k8s, " + err.Error())
8591
return err
8692
}
8793
err = p.clientset.CoreV1Client.Services(p.namespace).Delete(podName, &apiV1.DeleteOptions{})
88-
if err != nil {
89-
err = errors.New("send command service/delete to k8s, " + err.Error())
90-
return err
94+
switch {
95+
case err != nil && strings.Contains(err.Error(), "not found"):
96+
// service already deleted
97+
case err != nil:
98+
return errors.New("send command service/delete to k8s, " + err.Error())
9199
}
92100
return nil
93101
}

‎pool/strategy/kubernetes/strategy.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/satori/go.uuid"
99
"net"
1010
"time"
11+
"fmt"
1112
)
1213

1314
type Strategy struct {
@@ -25,14 +26,17 @@ func (s *Strategy) Reserve(desiredCaps capabilities.Capabilities) (pool.Node, er
2526
podName := "wd-node-" + uuid.NewV4().String()
2627
ts := time.Now().Unix()
2728
address := net.JoinHostPort(podName, nodeConfig.Params.Port)
28-
node := pool.NewNode(pool.NodeTypeKubernetes, address, pool.NodeStatusReserved, "", ts, ts, []capabilities.Capabilities{})
29+
node := pool.NewNode(podName, pool.NodeTypeKubernetes, address, pool.NodeStatusReserved, "", ts, ts, []capabilities.Capabilities{})
2930
err := s.storage.Add(*node, s.config.Limit)
3031
if err != nil {
3132
return pool.Node{}, errors.New("add node to storage, " + err.Error())
3233
}
3334
err = s.provider.Create(podName, nodeConfig.Params)
3435
if err != nil {
35-
_ = s.provider.Destroy(podName) // на случай если что то успело создасться
36+
go func(podName string) {
37+
time.Sleep(time.Minute * 2)
38+
_ = s.provider.Destroy(podName) // на случай если что то криво создалось
39+
}(podName)
3640
return pool.Node{}, errors.New("create node by provider, " + err.Error())
3741
}
3842
return *node, nil
@@ -43,11 +47,10 @@ func (s *Strategy) CleanUp(node pool.Node) error {
4347
if node.Type != pool.NodeTypeKubernetes {
4448
return strategy.ErrNotApplicable
4549
}
46-
hostName, _, err := net.SplitHostPort(node.Address)
47-
if err != nil {
48-
return errors.New("get hostname from node.Address, " + err.Error())
50+
if node.Key == "" {
51+
return fmt.Errorf("empty node key")
4952
}
50-
err = s.provider.Destroy(hostName)
53+
err := s.provider.Destroy(node.Key)
5154
if err != nil {
5255
return errors.New("destroy node by provider, " + err.Error())
5356
}
@@ -62,11 +65,10 @@ func (s *Strategy) FixNodeStatus(node pool.Node) error {
6265
if node.Type != pool.NodeTypeKubernetes {
6366
return strategy.ErrNotApplicable
6467
}
65-
hostName, _, err := net.SplitHostPort(node.Address)
66-
if err != nil {
67-
return errors.New("get hostname from node.Address, " + err.Error())
68+
if node.Key == "" {
69+
return fmt.Errorf("empty node key")
6870
}
69-
err = s.provider.Destroy(hostName)
71+
err := s.provider.Destroy(node.Key)
7072
if err != nil {
7173
return errors.New("destroy node by provider, " + err.Error())
7274
}

‎pool/strategy/kubernetes/strategy_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func TestStrategy_CleanUp_Positive(t *testing.T) {
7474
sm := new(pool.StorageMock)
7575
sm.On("Remove", mock.AnythingOfType("pool.Node")).Return(nil)
7676
s := Strategy{storage: sm, provider: pm}
77-
node := pool.Node{Type: pool.NodeTypeKubernetes, Address: "host:port"}
77+
node := pool.Node{Key: "valid_key", Type: pool.NodeTypeKubernetes}
7878
err := s.CleanUp(node)
7979
assert.Nil(t, err)
8080
}
@@ -86,9 +86,9 @@ func TestStrategy_CleanUp_Negative_NodeType(t *testing.T) {
8686
assert.Error(t, err, strategy.ErrNotApplicable)
8787
}
8888

89-
func TestStrategy_CleanUp_Negative_InvalidNodeAddress(t *testing.T) {
89+
func TestStrategy_CleanUp_Negative_EmptyNodeKey(t *testing.T) {
9090
s := Strategy{}
91-
node := pool.Node{Type: pool.NodeTypeKubernetes, Address: "invalid node address"}
91+
node := pool.Node{Key: "", Type: pool.NodeTypeKubernetes} // empty node key
9292
err := s.CleanUp(node)
9393
assert.NotNil(t, err)
9494
}
@@ -119,7 +119,7 @@ func TestStrategy_FixNodeStatus_Positive(t *testing.T) {
119119
sm := new(pool.StorageMock)
120120
sm.On("Remove", mock.AnythingOfType("pool.Node")).Return(nil)
121121
s := Strategy{storage: sm, provider: pm}
122-
node := pool.Node{Type: pool.NodeTypeKubernetes, Address: "host:port"}
122+
node := pool.Node{ Key: "valid_key", Type: pool.NodeTypeKubernetes}
123123
err := s.FixNodeStatus(node)
124124
assert.Nil(t, err)
125125
}
@@ -131,9 +131,9 @@ func TestStrategy_FixNodeStatus_Negative_NodeType(t *testing.T) {
131131
assert.Error(t, err, strategy.ErrNotApplicable)
132132
}
133133

134-
func TestStrategy_FixNodeStatus_Negative_InvalidNodeAddress(t *testing.T) {
134+
func TestStrategy_FixNodeStatus_Negative_EmptyNodeKey(t *testing.T) {
135135
s := Strategy{}
136-
node := pool.Node{Type: pool.NodeTypeKubernetes, Address: "invalid node address"}
136+
node := pool.Node{Key: "", Type: pool.NodeTypeKubernetes}
137137
err := s.FixNodeStatus(node)
138138
assert.NotNil(t, err)
139139
}

‎pool/strategyList_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestStrategyList_Reserve_PositiveDirectOrder(t *testing.T) {
2020
s1 := new(StrategyListMock)
2121
s1.On("Reserve", mock.AnythingOfType("capabilities.Capabilities")).Return(Node{}, strategy.ErrNotFound)
2222
s2 := new(StrategyListMock)
23-
expectedNode := *NewNode(NodeTypePersistent, "111", NodeStatusBusy, "", time.Now().Unix(), 0, []capabilities.Capabilities{})
23+
expectedNode := *NewNode("123", NodeTypePersistent, "111", NodeStatusBusy, "", time.Now().Unix(), 0, []capabilities.Capabilities{})
2424
s2.On("Reserve", mock.AnythingOfType("capabilities.Capabilities")).Return(expectedNode, nil)
2525

2626
sl := NewStrategyList([]StrategyInterface{s1, s2})
@@ -31,7 +31,7 @@ func TestStrategyList_Reserve_PositiveDirectOrder(t *testing.T) {
3131

3232
func TestStrategyList_Reserve_Positive_ReverseOrder(t *testing.T) {
3333
s1 := new(StrategyListMock)
34-
expectedNode := *NewNode(NodeTypePersistent, "111", NodeStatusBusy, "", time.Now().Unix(), 0, []capabilities.Capabilities{})
34+
expectedNode := *NewNode("123", NodeTypePersistent, "111", NodeStatusBusy, "", time.Now().Unix(), 0, []capabilities.Capabilities{})
3535
s1.On("Reserve", mock.AnythingOfType("capabilities.Capabilities")).Return(expectedNode, nil)
3636
s2 := new(StrategyListMock)
3737
s2.On("Reserve", mock.AnythingOfType("capabilities.Capabilities")).Return(Node{}, strategy.ErrNotFound)

‎storage/migrations/mysql/20180417152820-add_unique_key.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
-- +migrate Up
3+
ALTER TABLE `node` ADD COLUMN `key` varchar(255) NOT NULL AFTER `id`;
4+
5+
-- +migrate Down
6+
SIGNAL SQLSTATE '45000' SET message_text = 'Impossible down this migration';

0 commit comments

Comments
 (0)