Skip to content

Commit

Permalink
Change readiness check to Download a file instead of Stat
Browse files Browse the repository at this point in the history
  • Loading branch information
Anton-Kalpakchiev committed Nov 8, 2024
1 parent 6c87f36 commit 814aa97
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 56 deletions.
6 changes: 5 additions & 1 deletion lib/backend/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
Expand All @@ -24,4 +24,8 @@ const (
DefaultBufferGuard datasize.ByteSize = 10 * datasize.MB
DefaultConcurrency int = 10
DefaultListMaxKeys int = 250

// Name and namespace of an empty blob used to check readiness.
ReadinessNamespace string = "readiness/blob"
ReadinessName string = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
)
22 changes: 10 additions & 12 deletions lib/backend/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
package backend

import (
"bytes"
"errors"
"fmt"
"regexp"

"github.com/uber/kraken/lib/backend/backenderrors"
"github.com/uber/kraken/utils/bandwidth"
"github.com/uber/kraken/utils/log"

Expand All @@ -29,12 +29,10 @@ import (
var (
ErrNamespaceNotFound = errors.New("no matches for namespace")
)
const isReadyNamespace = "isReadyNamespace"
const isReadyName = "38a03d499119bc417b8a6a016f2cb4540b9f9cc0c13e4da42a73867120d3e908"

type backend struct {
regexp *regexp.Regexp
client Client
regexp *regexp.Regexp
client Client
mustReady bool
}

Expand All @@ -44,8 +42,8 @@ func newBackend(namespace string, c Client, mustReady bool) (*backend, error) {
return nil, fmt.Errorf("regexp: %s", err)
}
return &backend{
regexp: re,
client: c,
regexp: re,
client: c,
mustReady: mustReady,
}, nil
}
Expand Down Expand Up @@ -148,13 +146,13 @@ func (m *Manager) GetClient(namespace string) (Client, error) {
// IsReady returns whether the backends are ready (reachable).
// A backend must be explicitly configured as required for readiness to be checked.
func (m *Manager) IsReady() (bool, error) {
for _, b := range m.backends {
if !b.mustReady {
for _, backend := range m.backends {
if !backend.mustReady {
continue
}
_, err := b.client.Stat(isReadyNamespace, isReadyName)
if err != nil && err != backenderrors.ErrBlobNotFound {
return false, fmt.Errorf("backend for namespace %s not ready: %s", b.regexp.String(), err)
err := backend.client.Download(ReadinessNamespace, ReadinessName, bytes.NewBuffer([]byte{}))
if err != nil {
return false, fmt.Errorf("backend for namespace %s not ready: %s", backend.regexp.String(), err)
}
}
return true, nil
Expand Down
74 changes: 31 additions & 43 deletions lib/backend/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
package backend_test

import (
"bytes"
"errors"
"testing"

"github.com/uber-go/tally"
"github.com/uber/kraken/core"
. "github.com/uber/kraken/lib/backend"
"github.com/uber/kraken/lib/backend/backenderrors"
"github.com/uber/kraken/lib/backend/namepath"
Expand Down Expand Up @@ -160,47 +160,44 @@ func TestManagerBandwidth(t *testing.T) {
}

func TestManagerIsReady(t *testing.T) {
const isReadyNamespace = "isReadyNamespace"
const isReadyName = "38a03d499119bc417b8a6a016f2cb4540b9f9cc0c13e4da42a73867120d3e908"

n1 := "foo/*"
n2 := "bar/*"

tests := []struct {
name string
mustReady1 bool
mustReady2 bool
mockStat1Err error
mockStat2Err error
expectedRes bool
expectedErr error
name string
mustReady1 bool
mustReady2 bool
mockDownload1Err error
mockDownload2Err error
expectedRes bool
expectedErr error
}{
{
name: "both required, both pass (one with nil, one with NotFound)",
mustReady1: true,
mustReady2: true,
mockStat1Err: nil,
mockStat2Err: backenderrors.ErrBlobNotFound,
expectedRes: true,
expectedErr: nil,
name: "both required, both pass",
mustReady1: true,
mustReady2: true,
mockDownload1Err: nil,
mockDownload2Err: nil,
expectedRes: true,
expectedErr: nil,
},
{
name: "both required, only second fails",
mustReady1: true,
mustReady2: true,
mockStat1Err: nil,
mockStat2Err: errors.New("network error"),
expectedRes: false,
expectedErr: errors.New("backend for namespace bar/* not ready: network error"),
name: "both required, only second fails",
mustReady1: true,
mustReady2: true,
mockDownload1Err: nil,
mockDownload2Err: errors.New("network error"),
expectedRes: false,
expectedErr: errors.New("backend for namespace bar/* not ready: network error"),
},
{
name: "second required, only first fails",
mustReady1: false,
mustReady2: true,
mockStat1Err: errors.New("network error"),
mockStat2Err: backenderrors.ErrBlobNotFound,
expectedRes: true,
expectedErr: nil,
name: "second required, only first fails",
mustReady1: false,
mustReady2: true,
mockDownload1Err: backenderrors.ErrBlobNotFound,
mockDownload2Err: nil,
expectedRes: true,
expectedErr: nil,
},
}

Expand All @@ -215,17 +212,8 @@ func TestManagerIsReady(t *testing.T) {

m := ManagerFixture()

mockStat1 := &core.BlobInfo{}
mockStat2 := &core.BlobInfo{}
if tc.mockStat1Err != nil {
mockStat1 = nil
}
if tc.mockStat2Err != nil {
mockStat2 = nil
}

c1.EXPECT().Stat(isReadyNamespace, isReadyName).Return(mockStat1, tc.mockStat1Err).AnyTimes()
c2.EXPECT().Stat(isReadyNamespace, isReadyName).Return(mockStat2, tc.mockStat2Err).AnyTimes()
c1.EXPECT().Download(ReadinessNamespace, ReadinessName, bytes.NewBuffer([]byte{})).Return(tc.mockDownload1Err).AnyTimes()
c2.EXPECT().Download(ReadinessNamespace, ReadinessName, bytes.NewBuffer([]byte{})).Return(tc.mockDownload2Err).AnyTimes()

require.NoError(m.Register(n1, c1, tc.mustReady1))
require.NoError(m.Register(n2, c2, tc.mustReady2))
Expand Down

0 comments on commit 814aa97

Please sign in to comment.