Skip to content
This repository has been archived by the owner on May 6, 2021. It is now read-only.

Commit

Permalink
Issue #110 Making sure idler feature is also checked on timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
@hferentschik @vpavlin
hferentschik authored and vpavlin committed Feb 16, 2018
1 parent b7d28e3 commit f47714b
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 65 deletions.
19 changes: 18 additions & 1 deletion internal/idler/user_idler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@ import (
"github.com/fabric8-services/fabric8-jenkins-idler/internal/configuration"
"github.com/fabric8-services/fabric8-jenkins-idler/internal/model"
"github.com/fabric8-services/fabric8-jenkins-idler/internal/openshift/client"
"github.com/fabric8-services/fabric8-jenkins-idler/internal/toggles"
log "github.com/sirupsen/logrus"
"sync"
"time"
)

var logger = log.WithFields(log.Fields{"component": "user-idler"})

const (
bufferSize = 10
)
Expand All @@ -25,9 +28,10 @@ type UserIdler struct {
userChan chan model.User
user model.User
config configuration.Configuration
features toggles.Features
}

func NewUserIdler(user model.User, openShiftClient client.OpenShiftClient, config configuration.Configuration) *UserIdler {
func NewUserIdler(user model.User, openShiftClient client.OpenShiftClient, config configuration.Configuration, features toggles.Features) *UserIdler {
logEntry := log.WithFields(log.Fields{
"component": "user-idler",
"username": user.Name,
Expand All @@ -46,6 +50,7 @@ func NewUserIdler(user model.User, openShiftClient client.OpenShiftClient, confi
userChan: userChan,
user: user,
config: config,
features: features,
}
return &userIdler
}
Expand All @@ -57,6 +62,18 @@ func (idler *UserIdler) GetChannel() chan model.User {
// checkIdle verifies the state of conditions and decides if we should idle/unidle
// and performs the required action if needed
func (idler *UserIdler) checkIdle() error {
enabled, err := idler.features.IsIdlerEnabled(idler.user.ID)
if err != nil {
return err
}

if enabled {
logger.WithFields(log.Fields{"user": idler.user.Name, "uuid": idler.user.ID}).Debug("Idler enabled. Evaluating conditions.")
} else {
logger.WithFields(log.Fields{"user": idler.user.Name, "uuid": idler.user.ID}).Debug("Idler not enabled. Skipping idle check.")
return nil
}

eval := idler.Conditions.Eval(idler.user)

if eval {
Expand Down
28 changes: 6 additions & 22 deletions internal/openshift/client/openshift_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ type OpenShiftClient interface {
IsIdle(namespace string, service string) (int, error)
GetRoute(n string, s string) (r string, tls bool, err error)
GetApiURL() string
WatchBuilds(namespace string, buildType string, callback func(model.Object) (bool, error)) error
WatchDeploymentConfigs(namespace string, nsSuffix string, callback func(model.DCObject) (bool, error)) error
WatchBuilds(namespace string, buildType string, callback func(model.Object) error) error
WatchDeploymentConfigs(namespace string, nsSuffix string, callback func(model.DCObject) error) error
}

// OpenShift is a client for OpenShift API
Expand Down Expand Up @@ -289,7 +289,7 @@ func (o OpenShift) getProjects() (projects []string, err error) {
}

//WatchBuilds consumes stream of build events from OpenShift and calls callback to process them
func (o OpenShift) WatchBuilds(namespace string, buildType string, callback func(model.Object) (bool, error)) error {
func (o OpenShift) WatchBuilds(namespace string, buildType string, callback func(model.Object) error) error {
//Use a http client with disabled timeout
c := &http.Client{
Transport: &http.Transport{
Expand Down Expand Up @@ -339,22 +339,18 @@ func (o OpenShift) WatchBuilds(namespace string, buildType string, callback func
continue
}
logger.Infof("Handling Build change for user %s", o.Object.Metadata.Namespace)
ok, err := callback(o)
err = callback(o)
if err != nil {
logger.Errorf("Error from callback: %s", err)
continue
}

if ok {
logger.Debugf("Event summary: Build %s -> %s, %s/%s", o.Object.Metadata.Name, o.Object.Status.Phase, o.Object.Status.StartTimestamp, o.Object.Status.CompletionTimestamp)
}
}
logger.Debug("Fell out of loop for Build")
}
}

// WatchDeploymentConfigs consumes stream of DC events from OpenShift and calls callback to process them
func (o OpenShift) WatchDeploymentConfigs(namespace string, nsSuffix string, callback func(model.DCObject) (bool, error)) error {
func (o OpenShift) WatchDeploymentConfigs(namespace string, nsSuffix string, callback func(model.DCObject) error) error {
//Use a http client with disabled timeout
c := &http.Client{
Transport: &http.Transport{
Expand Down Expand Up @@ -407,23 +403,11 @@ func (o OpenShift) WatchDeploymentConfigs(namespace string, nsSuffix string, cal
}

logger.Infof("Handling DC change for user %s", o.Object.Metadata.Namespace)
ok, err := callback(o)
err = callback(o)
if err != nil {
logger.Errorf("Error from DC callback: %s", err)
continue
}

if ok {
// TODO - should this go away or at least be conditional?
// Get piece of status for debug info
c, err := o.Object.Status.GetByType("Available")
if err != nil {
logger.Error(err)
continue
}

logger.Debugf("Event summary: DeploymentConfig %s, %s/%s", o.Object.Metadata.Name, c.Status, c.LastUpdateTime)
}
}
logger.Debugf("Fall out od loop for watching DC")
}
Expand Down
48 changes: 11 additions & 37 deletions internal/openshift/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ var logger = log.WithFields(log.Fields{"component": "controller"})

// Controller defines the interface for watching the OpenShift cluster for changes.
type Controller interface {
HandleBuild(o model.Object) (bool, error)
HandleDeploymentConfig(dc model.DCObject) (bool, error)
HandleBuild(o model.Object) error
HandleDeploymentConfig(dc model.DCObject) error
GetUser(ns string) model.User
}

Expand Down Expand Up @@ -67,29 +67,16 @@ func (oc *OpenShiftController) GetUser(ns string) model.User {
// user structure with latest build info. NOTE: In most cases the only change in
// build object is stage timestamp, which we don't care about, so this function
// just does couple comparisons and returns
func (oc *OpenShiftController) HandleBuild(o model.Object) (bool, error) {
func (oc *OpenShiftController) HandleBuild(o model.Object) error {
ns := o.Object.Metadata.Namespace
logger.WithField("ns", ns).Infof("Processing build event %s", o.Object.Metadata.Name)

err := oc.createIfNotExist(o.Object.Metadata.Namespace)
if err != nil {
return false, err
return err
}

user := oc.userForNamespace(ns)
logger.WithFields(log.Fields{"ns": ns, "uuid": user.ID}).Debug("Checking if Idler is enabled")

enabled, err := oc.features.IsIdlerEnabled(user.ID)
if err != nil {
return false, err
}

if enabled {
logger.WithFields(log.Fields{"ns": ns, "uuid": user.ID}).Debug("Idler enabled")
} else {
logger.WithFields(log.Fields{"ns": ns, "uuid": user.ID}).Debug("Idler not enabled")
return false, nil
}

if oc.isActive(&o.Object) {
lastActive := user.ActiveBuild
Expand All @@ -116,40 +103,27 @@ func (oc *OpenShiftController) HandleBuild(o model.Object) (bool, error) {
oc.sendUserToIdler(ns, user)
}

return true, nil
return nil
}

// HandleDeploymentConfig processes new DC event collected from OpenShift and updates
// user structure with info about the changes in DC. NOTE: This is important for cases
// like reset tenant and update tenant when DC is updated and Jenkins starts because
// of ConfigChange or manual intervention.
func (oc *OpenShiftController) HandleDeploymentConfig(dc model.DCObject) (bool, error) {
func (oc *OpenShiftController) HandleDeploymentConfig(dc model.DCObject) error {
ns := dc.Object.Metadata.Namespace[:len(dc.Object.Metadata.Namespace)-len(jenkinsNamespaceSuffix)]
logger.WithField("ns", ns).Infof("Processing deployment config change event %s", dc.Object.Metadata.Name)

err := oc.createIfNotExist(ns)
if err != nil {
return false, err
return err
}

user := oc.userForNamespace(ns)
logger.WithFields(log.Fields{"ns": ns, "uuid": user.ID}).Debug("Checking if Idler is enabled")

enabled, err := oc.features.IsIdlerEnabled(user.ID)
if err != nil {
return false, err
}

if enabled {
logger.WithFields(log.Fields{"ns": ns, "uuid": user.ID}).Debug("Idler enabled")
} else {
logger.WithFields(log.Fields{"ns": ns, "uuid": user.ID}).Debug("Idler not enabled")
return false, nil
}

c, err := dc.Object.Status.GetByType(availableCond)
if err != nil {
return false, err
return err
}

// TODO Verify if we need Generation vs. ObservedGeneration
Expand All @@ -163,7 +137,7 @@ func (oc *OpenShiftController) HandleDeploymentConfig(dc model.DCObject) (bool,
// Also check if the event means that Jenkins just started (OS AvailableCondition.Status == true) and update time
status, err := strconv.ParseBool(c.Status)
if err != nil {
return false, err
return err
}

if status == true {
Expand All @@ -172,7 +146,7 @@ func (oc *OpenShiftController) HandleDeploymentConfig(dc model.DCObject) (bool,
oc.sendUserToIdler(ns, user)
}

return true, nil
return nil
}

// createIfNotExist check existence of a user in the map, initialise if it does not exist
Expand Down Expand Up @@ -200,7 +174,7 @@ func (oc *OpenShiftController) createIfNotExist(ns string) error {

newUser := model.NewUser(ti.Data[0].Id, ns, state == model.JenkinsRunning)
oc.users.Store(ns, newUser)
userIdler := idler.NewUserIdler(newUser, oc.openShiftClient, oc.config)
userIdler := idler.NewUserIdler(newUser, oc.openShiftClient, oc.config, oc.features)
oc.userChannels.Store(ns, userIdler.GetChannel())
userIdler.Run(oc.wg, oc.ctx, oc.cancel)

Expand Down
7 changes: 2 additions & 5 deletions internal/openshift/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"testing"

"context"
"fmt"
"github.com/fabric8-services/fabric8-jenkins-idler/internal/model"
"github.com/fabric8-services/fabric8-jenkins-idler/internal/openshift/client"
"github.com/fabric8-services/fabric8-jenkins-idler/internal/tenant"
Expand Down Expand Up @@ -50,9 +49,8 @@ func Test_handle_build(t *testing.T) {
Type: "MODIFIED",
}

ok, err := controller.HandleBuild(obj)
err := controller.HandleBuild(obj)
assert.NoError(t, err)
assert.True(t, ok, fmt.Sprintf("Namespace '%s' should be watched", obj.Object.Metadata.Namespace))
}

func Test_handle_deployment_config(t *testing.T) {
Expand All @@ -76,9 +74,8 @@ func Test_handle_deployment_config(t *testing.T) {
Type: "MODIFIED",
}

ok, err := controller.HandleDeploymentConfig(obj)
err := controller.HandleDeploymentConfig(obj)
assert.NoError(t, err)
assert.True(t, ok, fmt.Sprintf("Namespace '%s' should be watched", obj.Object.Metadata.Namespace))
}

func setUp(t *testing.T) {
Expand Down

0 comments on commit f47714b

Please sign in to comment.