Skip to content

Commit

Permalink
Account properly for no git config
Browse files Browse the repository at this point in the history
The *git.Repo state machine has a state for `RepoNoConfig` (meaning no
repo URL supplied), but hitherto it was not reachable, because the
constructor initialised the state to `RepoNew` (not cloned yet), and
there is no transition from there to `RepoNoConfig`.

We do want to distinguish the "no config" state from just not being
ready, however. This commit makes the state reachable, and accounts
for it in *git.Repo methods.

Ultimately this is visible via the API, where "no config" is now
treated as a reason for ListController results to be read-only.
squaremo committed Mar 13, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 8f8dc0c commit d5bfb67
Showing 3 changed files with 48 additions and 30 deletions.
9 changes: 5 additions & 4 deletions api/v6/api.go
Original file line number Diff line number Diff line change
@@ -23,10 +23,11 @@ type ImageStatus struct {
type ReadOnlyReason string

const (
ReadOnlyOK ReadOnlyReason = ""
ReadOnlyMissing ReadOnlyReason = "NotInRepo"
ReadOnlySystem ReadOnlyReason = "System"
ReadOnlyNoRepo ReadOnlyReason = "NoRepo"
ReadOnlyOK ReadOnlyReason = ""
ReadOnlyMissing ReadOnlyReason = "NotInRepo"
ReadOnlySystem ReadOnlyReason = "System"
ReadOnlyNoRepo ReadOnlyReason = "NoRepo"
ReadOnlyNotReady ReadOnlyReason = "NotReady"
)

type ControllerStatus struct {
18 changes: 10 additions & 8 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
@@ -76,35 +76,37 @@ func (d *Daemon) ListServices(ctx context.Context, namespace string) ([]v6.Contr
}

var services policy.ResourceMap
var noRepo bool
var globalReadOnly v6.ReadOnlyReason
err = d.WithClone(ctx, func(checkout *git.Checkout) error {
var err error
services, err = d.Manifests.ServicesWithPolicies(checkout.ManifestDir())
return err
})
switch {
case err == git.ErrNotReady:
noRepo = true
globalReadOnly = v6.ReadOnlyNotReady
case err == git.ErrNoConfig:
globalReadOnly = v6.ReadOnlyNoRepo
case err != nil:
return nil, errors.Wrap(err, "getting service policies")
}

var res []v6.ControllerStatus
for _, service := range clusterServices {
var readonly v6.ReadOnlyReason
var readOnly v6.ReadOnlyReason
policies, ok := services[service.ID]
switch {
case noRepo:
readonly = v6.ReadOnlyNoRepo
case globalReadOnly != "":
readOnly = globalReadOnly
case !ok:
readonly = v6.ReadOnlyMissing
readOnly = v6.ReadOnlyMissing
case service.IsSystem:
readonly = v6.ReadOnlySystem
readOnly = v6.ReadOnlySystem
}
res = append(res, v6.ControllerStatus{
ID: service.ID,
Containers: containers2containers(service.ContainersOrNil()),
ReadOnly: readonly,
ReadOnly: readOnly,
Status: service.Status,
Automated: policies.Contains(policy.Automated),
Locked: policies.Contains(policy.Locked),
51 changes: 33 additions & 18 deletions git/repo.go
Original file line number Diff line number Diff line change
@@ -31,9 +31,9 @@ type GitRepoStatus string

const (
RepoNoConfig GitRepoStatus = "unconfigured" // configuration is empty
RepoNew = "new" // no attempt made to clone it yet
RepoCloned = "cloned" // has been read (cloned); no attempt made to write
RepoReady = "ready" // has been written to, so ready to sync
RepoNew GitRepoStatus = "new" // no attempt made to clone it yet
RepoCloned GitRepoStatus = "cloned" // has been read (cloned); no attempt made to write
RepoReady GitRepoStatus = "ready" // has been written to, so ready to sync
)

// Remote points at a git repo somewhere.
@@ -57,9 +57,13 @@ type Repo struct {

// NewRepo constructs a repo mirror which will sync itself.
func NewRepo(origin Remote) *Repo {
status := RepoNew
if origin.URL == "" {
status = RepoNoConfig
}
r := &Repo{
origin: origin,
status: RepoNew,
status: status,
err: nil,
notify: make(chan struct{}, 1), // `1` so that Notify doesn't block
C: make(chan struct{}, 1), // `1` so we don't block on completing a refresh
@@ -109,30 +113,43 @@ func (r *Repo) Notify() {
}
}

// errorIfNotReady returns the appropriate error if the repo is not
// ready, and `nil` otherwise.
func (r *Repo) errorIfNotReady() error {
switch r.status {
case RepoReady:
return nil
case RepoNoConfig:
return ErrNoConfig
default:
return ErrNotReady
}
}

// Revision returns the revision (SHA1) of the ref passed in
func (r *Repo) Revision(ctx context.Context, ref string) (string, error) {
r.mu.RLock()
defer r.mu.RUnlock()
if r.dir == "" {
return "", ErrNotReady
if err := r.errorIfNotReady(); err != nil {
return "", err
}
return refRevision(ctx, r.dir, ref)
}

func (r *Repo) CommitsBefore(ctx context.Context, ref, path string) ([]Commit, error) {
r.mu.RLock()
defer r.mu.RUnlock()
if r.dir == "" {
return nil, ErrNotReady
if err := r.errorIfNotReady(); err != nil {
return nil, err
}
return onelinelog(ctx, r.dir, ref, path)
}

func (r *Repo) CommitsBetween(ctx context.Context, ref1, ref2, path string) ([]Commit, error) {
r.mu.RLock()
defer r.mu.RUnlock()
if r.dir == "" {
return nil, ErrNotReady
if err := r.errorIfNotReady(); err != nil {
return nil, err
}
return onelinelog(ctx, r.dir, ref1+".."+ref2, path)
}
@@ -154,12 +171,10 @@ func (r *Repo) Start(shutdown <-chan struct{}, done *sync.WaitGroup) error {

switch status {

// TODO(michael): I don't think this is a real status; perhaps
// have a no-op repo instead.
case RepoNoConfig:
// this is not going to change in the lifetime of this
// process
return ErrNoConfig
// process, so just exit.
return nil
case RepoNew:

rootdir, err := ioutil.TempDir(os.TempDir(), "flux-gitclone")
@@ -221,8 +236,8 @@ func (r *Repo) Refresh(ctx context.Context) error {
// could clone to another repo and pull there, then swap when complete.
r.mu.Lock()
defer r.mu.Unlock()
if r.status != RepoReady {
return ErrNotReady
if err := r.errorIfNotReady(); err != nil {
return err
}
if err := r.fetch(ctx); err != nil {
return err
@@ -276,8 +291,8 @@ func (r *Repo) fetch(ctx context.Context) error {
func (r *Repo) workingClone(ctx context.Context, ref string) (string, error) {
r.mu.RLock()
defer r.mu.RUnlock()
if r.dir == "" {
return "", ErrNotReady
if err := r.errorIfNotReady(); err != nil {
return "", err
}
working, err := ioutil.TempDir(os.TempDir(), "flux-working")
if err != nil {

0 comments on commit d5bfb67

Please sign in to comment.