Skip to content

Commit

Permalink
Merge pull request #90 from AlayaCare/collectWarnings
Browse files Browse the repository at this point in the history
collecting detailed errors and warnings
  • Loading branch information
nzin-alayacare authored Oct 21, 2024
2 parents 7be6ec7 + 01e49b1 commit dd60337
Show file tree
Hide file tree
Showing 19 changed files with 194 additions and 36 deletions.
35 changes: 34 additions & 1 deletion browser/goliac-ui/src/components/DashboardApp.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,35 @@

</el-table>
</el-row>
<el-row v-if="detailedErrors.length > 0 || detailedWarnings.length > 0">
<el-divider />
</el-row>
<el-row v-if="detailedErrors.length > 0">
<el-table
:data="detailedErrors"
:stripe="true"
:highlight-current-row="false"
>
<el-table-column prop="Errors" align="left" label="Errors">
<template #default="{row}">
<span>{{ row }}</span>
</template>
</el-table-column>
</el-table>
</el-row>
<el-row v-if="detailedWarnings.length > 0">
<el-table
:data="detailedWarnings"
:stripe="true"
:highlight-current-row="false"
>
<el-table-column prop="Warnings" align="left" label="Warnings">
<template #default="{row}">
<span>{{ row }}</span>
</template>
</el-table-column>
</el-table>
</el-row>
<el-row>
<el-divider />
</el-row>
Expand All @@ -38,7 +67,7 @@
<span> &nbsp; </span>
<el-button @click="flushcache">Flush cache</el-button>
</div>
</el-row>
</el-row>
<el-row>
<el-divider />
</el-row>
Expand Down Expand Up @@ -71,6 +100,8 @@
return {
flushcacheVisible: false,
statusTable: [],
detailedErrors: [],
detailedWarnings: [],
version: "",
};
},
Expand All @@ -82,6 +113,8 @@
Axios.get(`${API_URL}/status`).then(response => {
let status = response.data;
this.version = status.version;
this.detailedErrors = status.detailedErrors;
this.detailedWarnings = status.detailedWarnings;
this.statusTable = [
{
key: "Last Sync",
Expand Down
4 changes: 2 additions & 2 deletions cmd/goliac/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ branch can be passed by parameter or by defining GOLIAC_SERVER_GIT_BRANCH env va
if err != nil {
logrus.Fatalf("failed to create goliac: %s", err)
}
err = goliac.Apply(true, repo, branch, true)
err, _, _ = goliac.Apply(true, repo, branch, true)
if err != nil {
logrus.Errorf("Failed to plan: %v", err)
}
Expand Down Expand Up @@ -93,7 +93,7 @@ branch can be passed by parameter or by defining GOLIAC_SERVER_GIT_BRANCH env va
if err != nil {
logrus.Fatalf("failed to create goliac: %s", err)
}
err = goliac.Apply(false, repo, branch, true)
err, _, _ = goliac.Apply(false, repo, branch, true)
if err != nil {
logrus.Errorf("Failed to apply: %v", err)
}
Expand Down
8 changes: 8 additions & 0 deletions docs/api_docs/bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,14 @@ definitions:
x-omitempty: false
version:
type: string
detailedErrors:
type: array
items:
type: string
detailedWarnings:
type: array
items:
type: string
error:
type: object
required:
Expand Down
7 changes: 7 additions & 0 deletions docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,10 @@ To do so, as a Gitbub admin, you can go to
Note:
- When Goliac will run (and its cache expires), it will put back the ruleset. Usually the cache is set to 86400 seconds (ie 1 day).
- if you want to re-apply the ruleset quickly (when you have finished with your emergency chage), you can go to the Goliac UI and click on the `Flush cache` button, and then click on the `Re-Sync` button.

## How to resolve "not enough owners for team filename XXX" warning

This error is happening if a team does not have enough owners.
Indeed a team must have at least 2 owners to be able to review and merge PRs (and the only owner cannot approve its own PRs).

As an admin you should add more owners to the team.
2 changes: 1 addition & 1 deletion internal/entity/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (t *Team) Validate(dirname string, users map[string]*User) (error, []Warnin
// warnings

if len(t.Spec.Owners) < 2 {
warnings = append(warnings, fmt.Errorf("no enough owner for team filename %s/team.yaml", dirname))
warnings = append(warnings, fmt.Errorf("not enough owners for team filename %s/team.yaml", dirname))
}

return nil, warnings
Expand Down
29 changes: 15 additions & 14 deletions internal/goliac.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ const (
* It is used to load and validate a goliac repository and apply it to github.
*/
type Goliac interface {
// will run and apply the reconciliation
Apply(dryrun bool, repositoryUrl, branch string, forcesync bool) error
// will run and apply the reconciliation,
// it returns an error if something went wrong, and a detailed list of errors and warnings
Apply(dryrun bool, repositoryUrl, branch string, forcesync bool) (error, []error, []entity.Warning)

// will clone run the user-plugin to sync users, and will commit to the team repository
UsersUpdate(repositoryUrl, branch string, dryrun bool, force bool) error
Expand Down Expand Up @@ -77,42 +78,42 @@ func (g *GoliacImpl) FlushCache() {
g.remote.FlushCache()
}

func (g *GoliacImpl) Apply(dryrun bool, repositoryUrl, branch string, forcesync bool) error {
err := g.loadAndValidateGoliacOrganization(repositoryUrl, branch)
func (g *GoliacImpl) Apply(dryrun bool, repositoryUrl, branch string, forcesync bool) (error, []error, []entity.Warning) {
err, errs, warns := g.loadAndValidateGoliacOrganization(repositoryUrl, branch)
defer g.local.Close()
if err != nil {
return fmt.Errorf("failed to load and validate: %s", err)
return fmt.Errorf("failed to load and validate: %s", err), errs, warns
}
u, err := url.Parse(repositoryUrl)
if err != nil {
return fmt.Errorf("failed to parse %s: %v", repositoryUrl, err)
return fmt.Errorf("failed to parse %s: %v", repositoryUrl, err), errs, warns
}

teamsreponame := strings.TrimSuffix(path.Base(u.Path), filepath.Ext(path.Base(u.Path)))

err = g.applyToGithub(dryrun, teamsreponame, branch, forcesync)
if err != nil {
return err
return err, errs, warns
}
return nil
return nil, errs, warns
}

func (g *GoliacImpl) loadAndValidateGoliacOrganization(repositoryUrl, branch string) error {
func (g *GoliacImpl) loadAndValidateGoliacOrganization(repositoryUrl, branch string) (error, []error, []entity.Warning) {
var errs []error
var warns []entity.Warning
if strings.HasPrefix(repositoryUrl, "https://") || strings.HasPrefix(repositoryUrl, "git@") {
accessToken, err := g.githubClient.GetAccessToken()
if err != nil {
return err
return err, nil, nil
}

err = g.local.Clone(accessToken, repositoryUrl, branch)
if err != nil {
return fmt.Errorf("unable to clone: %v", err)
return fmt.Errorf("unable to clone: %v", err), nil, nil
}
repoconfig, err := g.local.LoadRepoConfig()
if err != nil {
return fmt.Errorf("unable to read goliac.yaml config file: %v", err)
return fmt.Errorf("unable to read goliac.yaml config file: %v", err), nil, nil
}
g.repoconfig = repoconfig

Expand All @@ -130,10 +131,10 @@ func (g *GoliacImpl) loadAndValidateGoliacOrganization(repositoryUrl, branch str
for _, err := range errs {
logrus.Error(err)
}
return fmt.Errorf("Not able to load and validate the goliac organization: see logs")
return fmt.Errorf("not able to load and validate the goliac organization: see logs"), errs, warns
}

return nil
return nil, errs, warns
}

/*
Expand Down
46 changes: 31 additions & 15 deletions internal/goliac_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ type GoliacServerImpl struct {
ready bool // when the server has finished to load the local configuration
lastSyncTime *time.Time
lastSyncError error
detailedErrors []error
detailedWarnings []entity.Warning
syncInterval int64 // in seconds time remaining between 2 sync
notificationService notification.NotificationService
}
Expand Down Expand Up @@ -419,17 +421,29 @@ func (g *GoliacServerImpl) GetUser(params app.GetUserParams) middleware.Responde

func (g *GoliacServerImpl) GetStatus(app.GetStatusParams) middleware.Responder {
s := models.Status{
LastSyncError: "",
LastSyncTime: "N/A",
NbRepos: int64(len(g.goliac.GetLocal().Repositories())),
NbTeams: int64(len(g.goliac.GetLocal().Teams())),
NbUsers: int64(len(g.goliac.GetLocal().Users())),
NbUsersExternal: int64(len(g.goliac.GetLocal().ExternalUsers())),
Version: config.GoliacBuildVersion,
LastSyncError: "",
LastSyncTime: "N/A",
NbRepos: int64(len(g.goliac.GetLocal().Repositories())),
NbTeams: int64(len(g.goliac.GetLocal().Teams())),
NbUsers: int64(len(g.goliac.GetLocal().Users())),
NbUsersExternal: int64(len(g.goliac.GetLocal().ExternalUsers())),
Version: config.GoliacBuildVersion,
DetailedErrors: make([]string, 0),
DetailedWarnings: make([]string, 0),
}
if g.lastSyncError != nil {
s.LastSyncError = g.lastSyncError.Error()
}
if g.detailedErrors != nil {
for _, err := range g.detailedErrors {
s.DetailedErrors = append(s.DetailedErrors, err.Error())
}
}
if g.detailedWarnings != nil {
for _, warn := range g.detailedWarnings {
s.DetailedWarnings = append(s.DetailedWarnings, warn.Error())
}
}
if g.lastSyncTime != nil {
s.LastSyncTime = g.lastSyncTime.UTC().Format("2006-01-02T15:04:05")
}
Expand Down Expand Up @@ -547,7 +561,7 @@ func (g *GoliacServerImpl) Serve() {
* - if the lobby is busy, it will do nothing
*/
func (g *GoliacServerImpl) triggerApply(forceresync bool) {
err, applied := g.serveApply(forceresync)
err, errs, warns, applied := g.serveApply(forceresync)
if !applied && err == nil {
// the run was skipped
g.syncInterval = config.Config.ServerApplyInterval
Expand All @@ -556,6 +570,8 @@ func (g *GoliacServerImpl) triggerApply(forceresync bool) {
g.lastSyncTime = &now
previousError := g.lastSyncError
g.lastSyncError = err
g.detailedErrors = errs
g.detailedWarnings = warns
// log the error only if it's a new one
if err != nil && (previousError == nil || err.Error() != previousError.Error()) {
logrus.Error(err)
Expand Down Expand Up @@ -604,14 +620,14 @@ func (g *GoliacServerImpl) StartRESTApi() (*restapi.Server, error) {
return server, nil
}

func (g *GoliacServerImpl) serveApply(forceresync bool) (error, bool) {
func (g *GoliacServerImpl) serveApply(forceresync bool) (error, []error, []entity.Warning, bool) {
// we want to run ApplyToGithub
// and queue one new run (the lobby) if a new run is asked
g.applyLobbyMutex.Lock()
// we already have a current run, and another waiting in the lobby
if g.applyLobby {
g.applyLobbyMutex.Unlock()
return nil, false
return nil, nil, nil, false
}

if !g.applyCurrent {
Expand Down Expand Up @@ -640,18 +656,18 @@ func (g *GoliacServerImpl) serveApply(forceresync bool) (error, bool) {
branch := config.Config.ServerGitBranch

if repo == "" {
return fmt.Errorf("GOLIAC_SERVER_GIT_REPOSITORY env variable not set"), false
return fmt.Errorf("GOLIAC_SERVER_GIT_REPOSITORY env variable not set"), nil, nil, false
}
if branch == "" {
return fmt.Errorf("GOLIAC_SERVER_GIT_BRANCH env variable not set"), false
return fmt.Errorf("GOLIAC_SERVER_GIT_BRANCH env variable not set"), nil, nil, false
}

// we are ready (to give local state, and to sync with remote)
g.ready = true

err := g.goliac.Apply(false, repo, branch, forceresync)
err, errs, warns := g.goliac.Apply(false, repo, branch, forceresync)
if err != nil {
return fmt.Errorf("failed to apply on branch %s: %s", branch, err), false
return fmt.Errorf("failed to apply on branch %s: %s", branch, err), errs, warns, false
}
return nil, true
return nil, errs, warns, true
}
4 changes: 2 additions & 2 deletions internal/goliac_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ type GoliacMock struct {
local engine.GoliacLocalResources
}

func (g *GoliacMock) Apply(dryrun bool, repo string, branch string, forceresync bool) error {
return nil
func (g *GoliacMock) Apply(dryrun bool, repo string, branch string, forceresync bool) (error, []error, []entity.Warning) {
return nil, nil, nil
}
func (g *GoliacMock) UsersUpdate(repositoryUrl, branch string, dryrun bool, force bool) error {
return nil
Expand Down
8 changes: 8 additions & 0 deletions swagger/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,14 @@ definitions:
x-omitempty: false
version:
type: string
detailedErrors:
type: array
items:
type: string
detailedWarnings:
type: array
items:
type: string

# Default Error
error:
Expand Down
5 changes: 5 additions & 0 deletions swagger_gen/models/collaborator_details.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions swagger_gen/models/repositories.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions swagger_gen/models/repository_details.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions swagger_gen/models/status.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit dd60337

Please sign in to comment.