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

Remote support #139

Merged
merged 11 commits into from
Feb 12, 2017
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
37 changes: 37 additions & 0 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ func generateNewJobMap() map[string]string {
"owner": "aj@ajvb.me",
}
}

func generateNewRemoteJobMap() map[string]interface{} {
return map[string]interface{}{
"name": "mock_remote_job",
"owner": "aj@ajvb.me",
"type": 1,
"remote_properties": map[string]string{
"url": "http://example.com",
},
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply gofmt to this


func generateJobAndCache() (*job.MemoryJobCache, *job.Job) {
cache := job.NewMockCache()
j := job.GetMockJobWithGenericSchedule()
Expand Down Expand Up @@ -70,6 +82,31 @@ func (a *ApiTestSuite) TestHandleAddJob() {
a.Equal(defaultOwner, retrievedJob.Owner)
a.Equal(w.Code, http.StatusCreated)
}

func (a *ApiTestSuite) TestHandleAddRemoteJob() {
t := a.T()
cache := job.NewMockCache()
jobMap := generateNewRemoteJobMap()
jobMap["owner"] = ""
defaultOwner := "aj+tester@ajvb.me"
handler := HandleAddJob(cache, defaultOwner)

jsonJobMap, err := json.Marshal(jobMap)
a.NoError(err)
w, req := setupTestReq(t, "POST", ApiJobPath, jsonJobMap)
handler(w, req)

var addJobResp AddJobResponse
err = json.Unmarshal(w.Body.Bytes(), &addJobResp)
a.NoError(err)
retrievedJob, err := cache.Get(addJobResp.Id)
a.NoError(err)
a.Equal(jobMap["name"], retrievedJob.Name)
a.NotEqual(jobMap["owner"], retrievedJob.Owner)
a.Equal(defaultOwner, retrievedJob.Owner)
a.Equal(w.Code, http.StatusCreated)
}

func (a *ApiTestSuite) TestHandleAddJobFailureBadJson() {
t := a.T()
cache := job.NewMockCache()
Expand Down
8 changes: 0 additions & 8 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,6 @@ dependencies:
pwd: ../gopath/src/${REPO_PATH}

test:
pre:
- go get github.com/axw/gocov/gocov github.com/mattn/goveralls golang.org/x/tools/cmd/cover github.com/bluesuncorp/overalls:
pwd: ../gopath/src/${REPO_PATH}
override:
- go test -race ./...:
pwd: ../gopath/src/${REPO_PATH}
- $GOPATH/bin/overalls -project=${REPO_PATH} -covermode=count -debug -ignore=.git,Godeps:
pwd: ../gopath/src/${REPO_PATH}
post:
- $GOPATH/bin/goveralls -coverprofile=overalls.coverprofile -service=circle-ci -repotoken=$COVERALLS_KEY:
pwd: ../gopath/src/${REPO_PATH}
68 changes: 54 additions & 14 deletions job/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/gob"
"errors"
"fmt"
"net/http"
"strconv"
"strings"
"sync"
Expand All @@ -13,23 +14,17 @@ import (
"github.com/ajvb/kala/utils/iso8601"

log "github.com/Sirupsen/logrus"
"github.com/mattn/go-shellwords"
"github.com/nu7hatch/gouuid"
)

var (
shParser = shellwords.NewParser()

RFC3339WithoutTimezone = "2006-01-02T15:04:05"

ErrInvalidJob = errors.New("Invalid Job. Job's must contain a Name and a Command field")
ErrInvalidJob = errors.New("Invalid Local Job. Job's must contain a Name and a Command field")
ErrInvalidRemoteJob = errors.New("Invalid Remote Job. Job's must contain a Name and a url field")
ErrInvalidJobType = errors.New("Invalid Job type. Types supported: 0 for local and 1 for remote")
)

func init() {
shParser.ParseEnv = true
shParser.ParseBacktick = true
}

type Job struct {
Name string `json:"name"`
Id string `json:"id"`
Expand Down Expand Up @@ -76,6 +71,12 @@ type Job struct {
// Meta data about successful and failed runs.
Metadata Metadata `json:"metadata"`

// Type of the job
JobType jobType `json:"type"`

// Custom properties for the remote job type
RemoteProperties RemoteProperties `json:"remote_properties"`

// Collection of Job Stats
Stats []*JobStat `json:"stats"`

Expand All @@ -86,6 +87,31 @@ type Job struct {
IsDone bool `json:"is_done"`
}

type jobType int

const (
LocalJob jobType = iota
RemoteJob
)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a comment describing RemoteProperties, can be the same as the above one. Mostly for the godocs generated html.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@levrado Good idea, but you should be able to just say iota rather than 0 + iota

// RemoteProperties Custom properties for the remote job type
type RemoteProperties struct {
Url string `json:"url"`
Method string `json:"method"`

// A body to attach to the http request
Body string `json:"body"`

// A list of headers to add to http request (e.g. [{"key": "charset", "value": "UTF-8"}])
Headers http.Header `json:"headers"`

// A timeout property for the http request in seconds
Timeout int `json:"timeout"`

// A list of expected response codes (e.g. [200, 201])
ExpectedResponseCodes []int `json:"expected_response_codes"`
}

type Metadata struct {
SuccessCount uint `json:"success_count"`
LastSuccess time.Time `json:"last_success"`
Expand Down Expand Up @@ -124,11 +150,10 @@ func (j *Job) Init(cache JobCache) error {
j.lock.Lock()
defer j.lock.Unlock()

// Job Validation
// TODO: Move this to a seperated method?
if j.Name == "" || j.Command == "" {
log.Errorf(ErrInvalidJob.Error())
return ErrInvalidJob
//validate job type and params
err := j.validation()
if err != nil {
return err
}

u4, err := uuid.NewV4()
Expand Down Expand Up @@ -432,3 +457,18 @@ func (j *Job) ShouldStartWaiting() bool {
}
return true
}

func (j *Job) validation() error {
var err error
if j.JobType == LocalJob && (j.Name == "" || j.Command == "") {
err = ErrInvalidJob
} else if j.JobType == RemoteJob && (j.Name == "" || j.RemoteProperties.Url == "") {
err = ErrInvalidRemoteJob
} else if j.JobType != LocalJob && j.JobType != RemoteJob {
err = ErrInvalidJobType
} else {
return nil
}
log.Errorf(err.Error())
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reduce code duplication by turning

		log.Errorf(ErrInvalidJob.Error())
		return ErrInvalidJob

into

err = ErrInvalidJob

then finish with

log.Errorf(err.Error())

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and use that same trick for every else if clause)

50 changes: 49 additions & 1 deletion job/job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"strings"
"testing"
"time"
"net/http/httptest"
"net/http"

"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -271,9 +273,9 @@ func TestJobEpsilon(t *testing.T) {

now := time.Now()

j.lock.RLock()
assert.Equal(t, j.Metadata.SuccessCount, uint(0))
assert.Equal(t, j.Metadata.ErrorCount, uint(2))
j.lock.RLock()
assert.WithinDuration(t, j.Metadata.LastError, now, 4*time.Second)
assert.WithinDuration(t, j.Metadata.LastAttemptedRun, now, 4*time.Second)
assert.True(t, j.Metadata.LastSuccess.IsZero())
Expand Down Expand Up @@ -927,3 +929,49 @@ func TestDependentJobsParentJobGetsDeleted(t *testing.T) {
assert.True(t, len(j.ParentJobs) == 1)
j.lock.RUnlock()
}

func TestRemoteJobRunner(t *testing.T) {
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, "Hello, client")
}))
defer testServer.Close()

mockRemoteJob := GetMockRemoteJob(RemoteProperties{
Url: testServer.URL,
})

cache := NewMockCache()
mockRemoteJob.Run(cache)
assert.True(t, mockRemoteJob.Metadata.SuccessCount == 1)
}

func TestRemoteJobBadStatus(t *testing.T) {
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "something failed", http.StatusInternalServerError)
}))
defer testServer.Close()

mockRemoteJob := GetMockRemoteJob(RemoteProperties{
Url: testServer.URL,
})

cache := NewMockCache()
mockRemoteJob.Run(cache)
assert.True(t, mockRemoteJob.Metadata.SuccessCount == 0)
}

func TestRemoteJobBadStatusSuccess(t *testing.T) {
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "something failed", http.StatusInternalServerError)
}))
defer testServer.Close()

mockRemoteJob := GetMockRemoteJob(RemoteProperties{
Url: testServer.URL,
ExpectedResponseCodes: []int{500},
})

cache := NewMockCache()
mockRemoteJob.Run(cache)
assert.True(t, mockRemoteJob.Metadata.SuccessCount == 1)
}
Loading