Skip to content

Commit

Permalink
Cleanup controller gen and proto gen
Browse files Browse the repository at this point in the history
Controller gen has an opinionated directory structure that doesn't
typically compliment Go guidelines on project structure. This change
tidies controller sources used for code gen to better align with Go
project structure best practice.

Additionally, the proto files that are generated and only used
internally have been moved to `internal`.

Signed-off-by: Chris Doherty <[email protected]>
  • Loading branch information
chrisdoherty4 committed Jan 10, 2023
1 parent d9f025f commit 13adfa3
Showing 43 changed files with 401 additions and 487 deletions.
1 change: 0 additions & 1 deletion .dockerignore

This file was deleted.

15 changes: 0 additions & 15 deletions .editorconfig

This file was deleted.

6 changes: 0 additions & 6 deletions .envrc

This file was deleted.

22 changes: 11 additions & 11 deletions Makefile
Original file line number Diff line number Diff line change
@@ -95,26 +95,26 @@ test: ## Run tests
source <($(SETUP_ENVTEST) use -p env) && $(GO) test -coverprofile=coverage.txt ./...

.PHONY: generate-proto
generate-proto: buf.gen.yaml buf.lock $(shell git ls-files 'protos/*/*.proto') _protoc
generate-proto: buf.gen.yaml buf.lock $(shell git ls-files '**/*.proto') _protoc
$(BUF) mod update
$(BUF) generate
$(GOFUMPT) -w protos/*/*.pb.*
$(GOFUMPT) -w internal/proto/*.pb.*

.PHONY: generate
generate: generate-proto generate-go generate-manifests ## Generate code, manifests etc.

.PHONY: generate-go
generate-go:
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate/boilerplate.generatego.txt" paths="./pkg/apis/..."
$(GOFUMPT) -w ./pkg/apis
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate/boilerplate.generatego.txt" paths="./api/..."
$(GOFUMPT) -w ./api

.PHONY: generate-manifests
generate-manifests: generate-crds generate-rbacs generate-server-rbacs ## Generate manifests e.g. CRD, RBAC etc.

.PHONY: generate-crds
generate-crds:
$(CONTROLLER_GEN) \
paths=./pkg/apis/... \
paths=./api/... \
crd:crdVersions=v1 \
rbac:roleName=manager-role \
output:crd:dir=./config/crd/bases \
@@ -123,17 +123,17 @@ generate-crds:
$(YAMLFMT) ./config/crd/bases/* ./config/webhook/*

.PHONY: generate-rbacs
generate-rbacs:
generate-rbacs:
$(CONTROLLER_GEN) \
paths=./pkg/controllers/... \
paths=./internal/controller/... \
output:rbac:dir=./config/rbac/ \
rbac:roleName=manager-role
$(YAMLFMT) ./config/rbac/*

.PHONY: generate-server-rbacs
generate-server-rbacs:
generate-server-rbacs:
$(CONTROLLER_GEN) \
paths=./server/... \
paths=./internal/server/... \
output:rbac:dir=./config/server-rbac \
rbac:roleName=server-role
$(YAMLFMT) ./config/server-rbac/*
@@ -168,9 +168,9 @@ check-generated: check-proto ## Check if generated files are up to date.

.PHONY: check-proto
check-proto: generate-proto
@git diff --no-ext-diff --quiet --exit-code -- protos/*/*.pb.* || (
@git diff --no-ext-diff --quiet --exit-code -- **/*.pb.* || (
echo "Protobuf files need to be regenerated!";
git diff --no-ext-diff --exit-code -- protos/*/*.pb.*
git diff --no-ext-diff --exit-code -- **/*.pb.*
)

.PHONY: verify
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
10 changes: 5 additions & 5 deletions cmd/tink-controller/main.go
Original file line number Diff line number Diff line change
@@ -10,8 +10,8 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/spf13/viper"
"github.com/tinkerbell/tink/pkg/controllers"
wfctrl "github.com/tinkerbell/tink/pkg/controllers/workflow"
"github.com/tinkerbell/tink/internal/controller"
"github.com/tinkerbell/tink/internal/workflow"
"k8s.io/client-go/tools/clientcmd"
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
)
@@ -72,16 +72,16 @@ func NewRootCommand(config *DaemonConfig, logger log.Logger) *cobra.Command {
if err != nil {
return err
}
options := controllers.GetControllerOptions()
options := controller.GetControllerOptions()
options.LeaderElectionNamespace = namespace
manager, err := controllers.NewManager(cfg, options)
manager, err := controller.NewManager(cfg, options)
if err != nil {
return err
}

return manager.RegisterControllers(
cmd.Context(),
wfctrl.NewController(manager.GetClient()),
workflow.NewController(manager.GetClient()),
).Start(cmd.Context())
},
}
4 changes: 2 additions & 2 deletions cmd/tink-worker/cmd/root.go
Original file line number Diff line number Diff line change
@@ -13,7 +13,7 @@ import (
"github.com/spf13/viper"
"github.com/tinkerbell/tink/cmd/tink-worker/worker"
"github.com/tinkerbell/tink/internal/client"
"github.com/tinkerbell/tink/protos/workflow"
"github.com/tinkerbell/tink/internal/workflow/proto"
)

const (
@@ -51,7 +51,7 @@ func NewRootCommand(version string, logger log.Logger) *cobra.Command {
if err != nil {
return err
}
workflowClient := workflow.NewWorkflowServiceClient(conn)
workflowClient := proto.NewWorkflowServiceClient(conn)

dockerClient, err := dockercli.NewClientWithOpts(dockercli.FromEnv, dockercli.WithAPIVersionNegotiation())
if err != nil {
26 changes: 13 additions & 13 deletions cmd/tink-worker/worker/container_manager.go
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ import (
"github.com/docker/docker/client"
"github.com/packethost/pkg/log"
"github.com/pkg/errors"
pb "github.com/tinkerbell/tink/protos/workflow"
"github.com/tinkerbell/tink/internal/proto"
)

const (
@@ -49,7 +49,7 @@ func NewContainerManager(logger log.Logger, cli DockerClient, registryDetails Re
return &containerManager{logger, cli, registryDetails}
}

func (m *containerManager) CreateContainer(ctx context.Context, cmd []string, wfID string, action *pb.WorkflowAction, captureLogs, privileged bool) (string, error) {
func (m *containerManager) CreateContainer(ctx context.Context, cmd []string, wfID string, action *proto.WorkflowAction, captureLogs, privileged bool) (string, error) {
l := m.getLogger(ctx)
config := &container.Config{
Image: path.Join(m.registryDetails.Registry, action.GetImage()),
@@ -98,10 +98,10 @@ func (m *containerManager) StartContainer(ctx context.Context, id string) error
return errors.Wrap(m.cli.ContainerStart(ctx, id, types.ContainerStartOptions{}), "DOCKER START")
}

func (m *containerManager) WaitForContainer(ctx context.Context, id string) (pb.State, error) {
func (m *containerManager) WaitForContainer(ctx context.Context, id string) (proto.State, error) {
// Inspect whether the container is in running state
if _, err := m.cli.ContainerInspect(ctx, id); err != nil {
return pb.State_STATE_FAILED, nil //nolint:nilerr // error is not nil, but it returns nil
return proto.State_STATE_FAILED, nil //nolint:nilerr // error is not nil, but it returns nil
}

// send API call to wait for the container completion
@@ -110,34 +110,34 @@ func (m *containerManager) WaitForContainer(ctx context.Context, id string) (pb.
select {
case status := <-wait:
if status.StatusCode == 0 {
return pb.State_STATE_SUCCESS, nil
return proto.State_STATE_SUCCESS, nil
}
return pb.State_STATE_FAILED, nil
return proto.State_STATE_FAILED, nil
case err := <-errC:
return pb.State_STATE_FAILED, err
return proto.State_STATE_FAILED, err
case <-ctx.Done():
return pb.State_STATE_TIMEOUT, ctx.Err()
return proto.State_STATE_TIMEOUT, ctx.Err()
}
}

func (m *containerManager) WaitForFailedContainer(ctx context.Context, id string, failedActionStatus chan pb.State) {
func (m *containerManager) WaitForFailedContainer(ctx context.Context, id string, failedActionStatus chan proto.State) {
l := m.getLogger(ctx)
// send API call to wait for the container completion
wait, errC := m.cli.ContainerWait(ctx, id, container.WaitConditionNotRunning)

select {
case status := <-wait:
if status.StatusCode == 0 {
failedActionStatus <- pb.State_STATE_SUCCESS
failedActionStatus <- proto.State_STATE_SUCCESS
return
}
failedActionStatus <- pb.State_STATE_FAILED
failedActionStatus <- proto.State_STATE_FAILED
case err := <-errC:
l.Error(err)
failedActionStatus <- pb.State_STATE_FAILED
failedActionStatus <- proto.State_STATE_FAILED
case <-ctx.Done():
l.Error(ctx.Err())
failedActionStatus <- pb.State_STATE_TIMEOUT
failedActionStatus <- proto.State_STATE_TIMEOUT
}
}

32 changes: 16 additions & 16 deletions cmd/tink-worker/worker/container_manager_test.go
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ import (
"github.com/docker/docker/client"
specs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/packethost/pkg/log"
pb "github.com/tinkerbell/tink/protos/workflow"
"github.com/tinkerbell/tink/internal/proto"
)

type fakeDockerClient struct {
@@ -91,7 +91,7 @@ func TestContainerManagerCreate(t *testing.T) {
cases := []struct {
name string
workflowName string
action *pb.WorkflowAction
action *proto.WorkflowAction
containerID string
registry string
clientErr error
@@ -100,7 +100,7 @@ func TestContainerManagerCreate(t *testing.T) {
{
name: "Happy Path",
workflowName: "saveTheRebelBase",
action: &pb.WorkflowAction{
action: &proto.WorkflowAction{
TaskName: "UseTheForce",
Name: "blow up the death star",
Image: "yav.in/4/forestmoon",
@@ -114,7 +114,7 @@ func TestContainerManagerCreate(t *testing.T) {
{
name: "create failure",
workflowName: "saveTheRebelBase",
action: &pb.WorkflowAction{
action: &proto.WorkflowAction{
TaskName: "UseTheForce",
Name: "blow up the death star",
Image: "yav.in/4/forestmoon",
@@ -206,41 +206,41 @@ func TestContainerManagerWait(t *testing.T) {
contextTimeout time.Duration
clientErr error
waitErr error
wantState pb.State
wantState proto.State
wantErr error
}{
{
name: "Happy Path",
containerID: "nomedalforchewie",
dockerResponse: 0,
wantState: pb.State_STATE_SUCCESS,
wantState: proto.State_STATE_SUCCESS,
},
{
name: "start failure",
containerID: "chewieDied",
dockerResponse: 1,
wantState: pb.State_STATE_FAILED,
wantState: proto.State_STATE_FAILED,
waitErr: nil,
},
{
name: "client wait failure",
containerID: "nomedalforchewie",
dockerResponse: 1,
wantState: pb.State_STATE_FAILED,
wantState: proto.State_STATE_FAILED,
waitErr: errors.New("Vader Won"),
wantErr: errors.New("Vader Won"),
},
{
name: "client inspect failure",
containerID: "nomedalforchewie",
wantState: pb.State_STATE_FAILED,
wantState: proto.State_STATE_FAILED,
clientErr: errors.New("inspect failed"),
wantErr: nil,
},
{
name: "client timeout",
containerID: "nomedalforchewie",
wantState: pb.State_STATE_TIMEOUT,
wantState: proto.State_STATE_TIMEOUT,
contextTimeout: time.Millisecond * 2,
waitErr: errors.New("Vader Won"),
wantErr: errors.New("context deadline exceeded"),
@@ -285,33 +285,33 @@ func TestContainerManagerWaitFailed(t *testing.T) {
contextTimeout time.Duration
waitTime time.Duration
clientErr error
wantState pb.State
wantState proto.State
}{
{
name: "Happy Path",
containerID: "nomedalforchewie",
dockerResponse: 0,
waitTime: 0,
wantState: pb.State_STATE_SUCCESS,
wantState: proto.State_STATE_SUCCESS,
},
{
name: "start failure",
containerID: "chewieDied",
dockerResponse: 1,
wantState: pb.State_STATE_FAILED,
wantState: proto.State_STATE_FAILED,
clientErr: nil,
},
{
name: "client wait failure",
containerID: "nomedalforchewie",
dockerResponse: 1,
wantState: pb.State_STATE_FAILED,
wantState: proto.State_STATE_FAILED,
clientErr: errors.New("Vader Won"),
},
{
name: "client timeout",
containerID: "nomedalforchewie",
wantState: pb.State_STATE_TIMEOUT,
wantState: proto.State_STATE_TIMEOUT,
waitTime: time.Millisecond * 20,
contextTimeout: time.Millisecond * 10,
clientErr: errors.New("Vader Won"),
@@ -327,7 +327,7 @@ func TestContainerManagerWaitFailed(t *testing.T) {
if tc.contextTimeout == 0 {
ctx = context.Background()
}
failedChan := make(chan pb.State)
failedChan := make(chan proto.State)
go mgr.WaitForFailedContainer(ctx, tc.containerID, failedChan)
got := <-failedChan

Loading

0 comments on commit 13adfa3

Please sign in to comment.