Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor api package to an interface #1020

Merged
merged 4 commits into from
May 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 12 additions & 37 deletions agent/agent_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ type AgentWorkerConfig struct {
// Whether to set debug in the job
Debug bool

// The endpoint that should be used when communicating with the API
Endpoint string

// Whether to disable http for the API
DisableHTTP2 bool

// The configuration of the agent from the CLI
AgentConfiguration AgentConfiguration
}
Expand All @@ -37,7 +31,7 @@ type AgentWorker struct {
lastPing, lastHeartbeat int64

// The API Client used when this agent is communicating with the API
apiClient *api.Client
apiClient APIClient

// The logger instance to use
logger logger.Logger
Expand Down Expand Up @@ -68,26 +62,12 @@ type AgentWorker struct {
}

// Creates the agent worker and initializes it's API Client
func NewAgentWorker(l logger.Logger, a *api.AgentRegisterResponse, m *metrics.Collector, c AgentWorkerConfig) *AgentWorker {
var endpoint string
if a.Endpoint != "" {
endpoint = a.Endpoint
} else {
endpoint = c.Endpoint
}

// Create an APIClient with the agent's access token
apiClient := NewAPIClient(l, APIClientConfig{
Endpoint: endpoint,
Token: a.AccessToken,
DisableHTTP2: c.DisableHTTP2,
})

func NewAgentWorker(l logger.Logger, a *api.AgentRegisterResponse, m *metrics.Collector, apiClient APIClient, c AgentWorkerConfig) *AgentWorker {
return &AgentWorker{
logger: l,
agent: a,
metricsCollector: m,
apiClient: apiClient,
apiClient: apiClient.FromAgentRegisterResponse(a),
debug: c.Debug,
agentConfiguration: c.AgentConfiguration,
stop: make(chan struct{}),
Expand Down Expand Up @@ -257,7 +237,7 @@ func (a *AgentWorker) Connect() error {
a.UpdateProcTitle("connecting")

return retry.Do(func(s *retry.Stats) error {
_, err := a.apiClient.Agents.Connect()
_, err := a.apiClient.Connect()
if err != nil {
a.logger.Warn("%s (%s)", err, s)
}
Expand All @@ -273,7 +253,7 @@ func (a *AgentWorker) Heartbeat() error {

// Retry the heartbeat a few times
err = retry.Do(func(s *retry.Stats) error {
beat, _, err = a.apiClient.Heartbeats.Beat()
beat, _, err = a.apiClient.Heartbeat()
if err != nil {
a.logger.Warn("%s (%s)", err, s)
}
Expand All @@ -297,7 +277,7 @@ func (a *AgentWorker) Ping() (*api.Job, error) {
// Update the proc title
a.UpdateProcTitle("pinging")

ping, _, err := a.apiClient.Pings.Get()
ping, _, err := a.apiClient.Ping()
if err != nil {
// Get the last ping time to the nearest microsecond
lastPing := time.Unix(atomic.LoadInt64(&a.lastPing), 0)
Expand All @@ -312,15 +292,11 @@ func (a *AgentWorker) Ping() (*api.Job, error) {

// Should we switch endpoints?
if ping.Endpoint != "" && ping.Endpoint != a.agent.Endpoint {
newAPIClient := a.apiClient.FromPing(ping)

// Before switching to the new one, do a ping test to make sure it's
// valid. If it is, switch and carry on, otherwise ignore the switch
// for now.
newAPIClient := NewAPIClient(a.logger, APIClientConfig{
Endpoint: ping.Endpoint,
Token: a.agent.AccessToken,
})

newPing, _, err := newAPIClient.Pings.Get()
newPing, _, err := newAPIClient.Ping()
if err != nil {
a.logger.Warn("Failed to ping the new endpoint %s - ignoring switch for now (%s)", ping.Endpoint, err)
} else {
Expand Down Expand Up @@ -364,7 +340,7 @@ func (a *AgentWorker) AcceptAndRun(job *api.Job) error {
var accepted *api.Job
err := retry.Do(func(s *retry.Stats) error {
var err error
accepted, _, err = a.apiClient.Jobs.Accept(job)
accepted, _, err = a.apiClient.AcceptJob(job)
if err != nil {
if api.IsRetryableError(err) {
a.logger.Warn("%s (%s)", err, s)
Expand Down Expand Up @@ -395,9 +371,8 @@ func (a *AgentWorker) AcceptAndRun(job *api.Job) error {
}()

// Now that the job has been accepted, we can start it.
a.jobRunner, err = NewJobRunner(a.logger, jobMetricsScope, a.agent, accepted, JobRunnerConfig{
a.jobRunner, err = NewJobRunner(a.logger, jobMetricsScope, a.agent, accepted, a.apiClient, JobRunnerConfig{
Debug: a.debug,
Endpoint: accepted.Endpoint,
AgentConfiguration: a.agentConfiguration,
})

Expand All @@ -422,7 +397,7 @@ func (a *AgentWorker) Disconnect() error {
// Update the proc title
a.UpdateProcTitle("disconnecting")

_, err := a.apiClient.Agents.Disconnect()
_, err := a.apiClient.Disconnect()
if err != nil {
a.logger.Warn("There was an error sending the disconnect API call to Buildkite. If this agent still appears online, you may have to manually stop it (%s)", err)
}
Expand Down
34 changes: 34 additions & 0 deletions agent/api.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Created by interfacer; DO NOT EDIT

package agent

import (
"github.com/buildkite/agent/api"
)

// APIClient is an interface generated for "github.com/buildkite/agent/api.Client".
type APIClient interface {
AcceptJob(*api.Job) (*api.Job, *api.Response, error)
Annotate(string, *api.Annotation) (*api.Response, error)
Config() api.Config
Connect() (*api.Response, error)
CreateArtifacts(string, *api.ArtifactBatch) (*api.ArtifactBatchCreateResponse, *api.Response, error)
Disconnect() (*api.Response, error)
ExistsMetaData(string, string) (*api.MetaDataExists, *api.Response, error)
FinishJob(*api.Job) (*api.Response, error)
FromAgentRegisterResponse(*api.AgentRegisterResponse) *api.Client
FromPing(*api.Ping) *api.Client
GetJobState(string) (*api.JobState, *api.Response, error)
GetMetaData(string, string) (*api.MetaData, *api.Response, error)
Heartbeat() (*api.Heartbeat, *api.Response, error)
Ping() (*api.Ping, *api.Response, error)
Register(*api.AgentRegisterRequest) (*api.AgentRegisterResponse, *api.Response, error)
SaveHeaderTimes(string, *api.HeaderTimes) (*api.Response, error)
SearchArtifacts(string, *api.ArtifactSearchOptions) ([]*api.Artifact, *api.Response, error)
SetMetaData(string, *api.MetaData) (*api.Response, error)
StartJob(*api.Job) (*api.Response, error)
StepUpdate(string, *api.StepUpdate) (*api.Response, error)
UpdateArtifacts(string, map[string]string) (*api.Response, error)
UploadChunk(string, *api.Chunk) (*api.Response, error)
UploadPipeline(string, *api.Pipeline) (*api.Response, error)
}
70 changes: 0 additions & 70 deletions agent/api_client.go

This file was deleted.

6 changes: 3 additions & 3 deletions agent/artifact_batch_creator.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ type ArtifactBatchCreator struct {
logger logger.Logger

// The APIClient that will be used when uploading jobs
apiClient *api.Client
apiClient APIClient
}

func NewArtifactBatchCreator(l logger.Logger, ac *api.Client, c ArtifactBatchCreatorConfig) *ArtifactBatchCreator {
func NewArtifactBatchCreator(l logger.Logger, ac APIClient, c ArtifactBatchCreatorConfig) *ArtifactBatchCreator {
return &ArtifactBatchCreator{
logger: l,
conf: c,
Expand Down Expand Up @@ -67,7 +67,7 @@ func (a *ArtifactBatchCreator) Create() ([]*api.Artifact, error) {

// Retry the batch upload a couple of times
err = retry.Do(func(s *retry.Stats) error {
creation, resp, err = a.apiClient.Artifacts.Create(a.conf.JobID, batch)
creation, resp, err = a.apiClient.CreateArtifacts(a.conf.JobID, batch)
if resp != nil && (resp.StatusCode == 401 || resp.StatusCode == 404 || resp.StatusCode == 500) {
s.Break()
}
Expand Down
18 changes: 10 additions & 8 deletions agent/artifact_downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"path/filepath"
"strings"

"github.com/buildkite/agent/api"
"github.com/buildkite/agent/logger"
"github.com/buildkite/agent/pool"
)
Expand All @@ -25,6 +24,9 @@ type ArtifactDownloaderConfig struct {

// Where we'll be downloading artifacts to
Destination string

// Whether to show HTTP debugging
DebugHTTP bool
}

type ArtifactDownloader struct {
Expand All @@ -34,11 +36,11 @@ type ArtifactDownloader struct {
// The logger instance to use
logger logger.Logger

// The *api.Client that will be used when uploading jobs
apiClient *api.Client
// The APIClient that will be used when uploading jobs
apiClient APIClient
}

func NewArtifactDownloader(l logger.Logger, ac *api.Client, c ArtifactDownloaderConfig) ArtifactDownloader {
func NewArtifactDownloader(l logger.Logger, ac APIClient, c ArtifactDownloaderConfig) ArtifactDownloader {
return ArtifactDownloader{
logger: l,
apiClient: ac,
Expand Down Expand Up @@ -90,31 +92,31 @@ func (a *ArtifactDownloader) Download() error {
Bucket: artifact.UploadDestination,
Destination: downloadDestination,
Retries: 5,
DebugHTTP: a.apiClient.DebugHTTP,
DebugHTTP: a.conf.DebugHTTP,
}).Start()
} else if strings.HasPrefix(artifact.UploadDestination, "gs://") {
err = NewGSDownloader(a.logger, GSDownloaderConfig{
Path: artifact.Path,
Bucket: artifact.UploadDestination,
Destination: downloadDestination,
Retries: 5,
DebugHTTP: a.apiClient.DebugHTTP,
DebugHTTP: a.conf.DebugHTTP,
}).Start()
} else if strings.HasPrefix(artifact.UploadDestination, "rt://") {
err = NewArtifactoryDownloader(a.logger, ArtifactoryDownloaderConfig{
Path: artifact.Path,
Repository: artifact.UploadDestination,
Destination: downloadDestination,
Retries: 5,
DebugHTTP: a.apiClient.DebugHTTP,
DebugHTTP: a.conf.DebugHTTP,
}).Start()
} else {
err = NewDownload(a.logger, http.DefaultClient, DownloadConfig{
URL: artifact.URL,
Path: artifact.Path,
Destination: downloadDestination,
Retries: 5,
DebugHTTP: a.apiClient.DebugHTTP,
DebugHTTP: a.conf.DebugHTTP,
}).Start()
}

Expand Down
6 changes: 4 additions & 2 deletions agent/artifact_downloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package agent

import (
"fmt"
"github.com/buildkite/agent/logger"
"net/http"
"net/http/httptest"
"testing"

"github.com/buildkite/agent/api"
"github.com/buildkite/agent/logger"
)

func TestArtifactDownloaderConnectsToEndpoint(t *testing.T) {
Expand All @@ -27,7 +29,7 @@ func TestArtifactDownloaderConnectsToEndpoint(t *testing.T) {
}))
defer server.Close()

ac := NewAPIClient(logger.Discard, APIClientConfig{
ac := api.NewClient(logger.Discard, api.Config{
Endpoint: server.URL,
Token: `llamasforever`,
})
Expand Down
6 changes: 3 additions & 3 deletions agent/artifact_searcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ type ArtifactSearcher struct {
logger logger.Logger

// The APIClient that will be used when uploading jobs
apiClient *api.Client
apiClient APIClient

// The ID of the Build that these artifacts belong to
buildID string
}

func NewArtifactSearcher(l logger.Logger, ac *api.Client, buildID string) *ArtifactSearcher {
func NewArtifactSearcher(l logger.Logger, ac APIClient, buildID string) *ArtifactSearcher {
return &ArtifactSearcher{
logger: l,
apiClient: ac,
Expand All @@ -31,7 +31,7 @@ func (a *ArtifactSearcher) Search(query string, scope string) ([]*api.Artifact,
a.logger.Info("Searching for artifacts: \"%s\" within step: \"%s\"", query, scope)
}

artifacts, _, err := a.apiClient.Artifacts.Search(a.buildID, &api.ArtifactSearchOptions{
artifacts, _, err := a.apiClient.SearchArtifacts(a.buildID, &api.ArtifactSearchOptions{
Query: query,
Scope: scope,
})
Expand Down
Loading