Skip to content

Commit

Permalink
Generalize the hardware client interface (#81)
Browse files Browse the repository at this point in the history
### Context

To compliment the [Kubeification of Tinkerbell's back-end](https://github.com/tinkerbell/proposals/tree/main/proposals/0026) we need to provide a mechanism for supplying hardware data from the Kubernetes API Server in Hegel.

### Changes

The hardware client is used to interact with a Cacher service and the Tink service. It embodies gRPC specific types and an ambiguous All() call that are difficult to model for a Kubernetes data provider.

This change alters the interface to model more generic behavior and removes gRPC specific types paving the path for implementing a Kubernetes data provider client that can plug into existing business logic.

This change doesn't alter any service behavior.
  • Loading branch information
mergify[bot] authored Apr 8, 2022
2 parents e570bc4 + 06a4519 commit 782db1c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 39 deletions.
54 changes: 24 additions & 30 deletions hardware/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,25 @@ import (
tinkClient "github.com/tinkerbell/tink/client"
tpkg "github.com/tinkerbell/tink/pkg"
tink "github.com/tinkerbell/tink/protos/hardware"
"google.golang.org/grpc"
)

var (
dataModelVersion = env.Get("DATA_MODEL_VERSION")
facility = flag.String("facility", env.Get("HEGEL_FACILITY", "onprem"), "The facility we are running in (mostly to connect to cacher)")
)

// Client acts as the messenger between Hegel and Cacher/Tink.
// Client defines the behaviors for interacting with hardware data providers.
type Client interface {
All(ctx context.Context, opts ...grpc.CallOption) (AllClient, error)
ByIP(ctx context.Context, id string, opts ...grpc.CallOption) (Hardware, error)
Watch(ctx context.Context, id string, opts ...grpc.CallOption) (Watcher, error)
}
// IsHealthy reports whether the client is connected and can retrieve hardware data from the data provider.
IsHealthy(ctx context.Context) bool

// ByIP retrieves hardware data by its IP address.
ByIP(ctx context.Context, ip string) (Hardware, error)

type AllClient interface{}
// Watch creates a subscription to a hardware identified by id such that updates to the hardware data are
// pushed to the stream.
Watch(ctx context.Context, id string) (Watcher, error)
}

// Hardware is the interface for Cacher/Tink hardware types.
type Hardware interface {
Expand Down Expand Up @@ -102,68 +105,59 @@ func NewClient() (Client, error) {
return hg, nil
}

// All retrieves all the pieces of hardware stored in Cacher.
func (hg clientCacher) All(ctx context.Context, opts ...grpc.CallOption) (AllClient, error) {
in := &cacher.Empty{}
all, err := hg.client.All(ctx, in, opts...)
if err != nil {
return nil, err
}
return all, nil
func (hg clientCacher) IsHealthy(ctx context.Context) bool {
_, err := hg.client.All(ctx, &cacher.Empty{})
return err == nil
}

// ByIP retrieves from Cacher the piece of hardware with the specified IP.
func (hg clientCacher) ByIP(ctx context.Context, ip string, opts ...grpc.CallOption) (Hardware, error) {
func (hg clientCacher) ByIP(ctx context.Context, ip string) (Hardware, error) {
in := &cacher.GetRequest{
IP: ip,
}
hw, err := hg.client.ByIP(ctx, in, opts...)
hw, err := hg.client.ByIP(ctx, in)
if err != nil {
return nil, err
}
return &Cacher{hw}, nil
}

// Watch returns a Cacher watch client on the hardware with the specified ID.
func (hg clientCacher) Watch(ctx context.Context, id string, opts ...grpc.CallOption) (Watcher, error) {
func (hg clientCacher) Watch(ctx context.Context, id string) (Watcher, error) {
in := &cacher.GetRequest{
ID: id,
}
w, err := hg.client.Watch(ctx, in, opts...)
w, err := hg.client.Watch(ctx, in)
if err != nil {
return nil, err
}
return &watcherCacher{w}, nil
}

// All retrieves all the pieces of hardware stored in Cacher.
func (hg clientTinkerbell) All(ctx context.Context, opts ...grpc.CallOption) (AllClient, error) {
in := &tink.Empty{}
all, err := hg.client.All(ctx, in, opts...)
if err != nil {
return nil, err
}
return all, nil
func (hg clientTinkerbell) IsHealthy(ctx context.Context) bool {
_, err := hg.client.All(ctx, &tink.Empty{})
return err == nil
}

// ByIP retrieves from Tink the piece of hardware with the specified IP.
func (hg clientTinkerbell) ByIP(ctx context.Context, ip string, opts ...grpc.CallOption) (Hardware, error) {
func (hg clientTinkerbell) ByIP(ctx context.Context, ip string) (Hardware, error) {
in := &tink.GetRequest{
Ip: ip,
}
hw, err := hg.client.ByIP(ctx, in, opts...)
hw, err := hg.client.ByIP(ctx, in)
if err != nil {
return nil, err
}
return &Tinkerbell{hw}, nil
}

// Watch returns a Tink watch client on the hardware with the specified ID.
func (hg clientTinkerbell) Watch(ctx context.Context, id string, opts ...grpc.CallOption) (Watcher, error) {
func (hg clientTinkerbell) Watch(ctx context.Context, id string) (Watcher, error) {
in := &tink.GetRequest{
Id: id,
}
w, err := hg.client.DeprecatedWatch(ctx, in, opts...)
w, err := hg.client.DeprecatedWatch(ctx, in)
if err != nil {
return nil, err
}
Expand Down
9 changes: 4 additions & 5 deletions hardware/mock/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/packethost/cacher/protos/cacher"
"github.com/tinkerbell/hegel/hardware"
"google.golang.org/grpc"
)

// HardwareClient is a mock implementation of the hardwareGetter interface
Expand All @@ -17,16 +16,16 @@ type HardwareClient struct {
Data string
}

func (hg HardwareClient) All(_ context.Context, _ ...grpc.CallOption) (hardware.AllClient, error) {
return nil, nil
func (hg HardwareClient) IsHealthy(context.Context) bool {
return true
}

// ByIP mocks the retrieval a piece of hardware from tink/cacher by ip
// In order to simulate the process of finding the piece of hardware that matches the IP provided in the get request without
// having to parse the (mock) hardware data `HardwareClient.Data`, the process has been simplified to only match with the constant `UserIP`.
// Given any other IP inside the get request, ByIP will return an empty piece of hardware regardless of whether or not the IP
// actually matches the IP inside `Data`.
func (hg HardwareClient) ByIP(_ context.Context, ip string, _ ...grpc.CallOption) (hardware.Hardware, error) {
func (hg HardwareClient) ByIP(_ context.Context, ip string) (hardware.Hardware, error) {
var hw hardware.Hardware
dataModelVersion := os.Getenv("DATA_MODEL_VERSION")
switch dataModelVersion {
Expand All @@ -52,7 +51,7 @@ func (hg HardwareClient) ByIP(_ context.Context, ip string, _ ...grpc.CallOption
return hw, nil
}

func (hg HardwareClient) Watch(_ context.Context, _ string, _ ...grpc.CallOption) (hardware.Watcher, error) {
func (hg HardwareClient) Watch(context.Context, string) (hardware.Watcher, error) {
return nil, nil
}

Expand Down
8 changes: 4 additions & 4 deletions http-server/http_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ func checkHardwareClientHealth() {
// this will have to do.
// Note that we don't do anything with the stream (we don't read from it)
var isHardwareClientAvailableTemp bool
ctx, cancel := context.WithCancel(context.Background())
_, err := hegelServer.HardwareClient().All(ctx) // checks for tink health as well
if err == nil {

ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
if hegelServer.HardwareClient().IsHealthy(ctx) {
isHardwareClientAvailableTemp = true
}
cancel()
Expand All @@ -118,6 +118,6 @@ func checkHardwareClientHealth() {
metrics.CacherConnected.Set(0)
metrics.CacherHealthcheck.WithLabelValues("false").Inc()
metrics.Errors.WithLabelValues("cacher", "healthcheck").Inc()
logger.With("status", isHardwareClientAvailableTemp).Error(err)
logger.With("status", isHardwareClientAvailableTemp).Error(errors.New("client reported unhealthy"))
}
}

0 comments on commit 782db1c

Please sign in to comment.