From 1e985e560fdef0c1132fbc2d55ef30441302f5c9 Mon Sep 17 00:00:00 2001 From: Pradeep Kumar Date: Mon, 28 Jan 2019 14:15:19 +0530 Subject: [PATCH] Allow the idler to exclude some deployments from idling Adds a new api to enable/disable idler for a set of users. Idler can be disbled for a user as follows ``` curl -H "Content-Type: application/json" \ -X POST \ --data '{"disable":["pradkuma-preview2"],"enable":[]}' \ http://idler:8080/api/idler/userstatus/ ``` Use `GET` to fetch the list of currently idled users ``` curl http://idler:8080/api/idler/userstatus/ ``` Resolves - https://github.com/fabric8-services/fabric8-jenkins-idler/issues/232 --- cmd/fabric8-jenkins-idler/idler.go | 23 ++++++--- internal/api/api.go | 59 ++++++++++++++++------ internal/model/stringset.go | 29 +++++++++++ internal/model/stringset_test.go | 61 +++++++++++++++++++++++ internal/openshift/controller.go | 19 ++++++- internal/openshift/controller_test.go | 3 +- internal/router/router.go | 9 ++-- internal/router/router_test.go | 31 ++++++++---- internal/testutils/mock/mock_idler_api.go | 24 ++++++--- 9 files changed, 211 insertions(+), 47 deletions(-) create mode 100644 internal/model/stringset.go create mode 100644 internal/model/stringset_test.go diff --git a/cmd/fabric8-jenkins-idler/idler.go b/cmd/fabric8-jenkins-idler/idler.go index 5c33d1d..13137c3 100644 --- a/cmd/fabric8-jenkins-idler/idler.go +++ b/cmd/fabric8-jenkins-idler/idler.go @@ -37,6 +37,8 @@ type Idler struct { tenantService tenant.Service clusterView cluster.View config configuration.Configuration + disabledUsers *model.StringSet + userIdlers *openshift.UserIdlerMap } // struct used to pass in cancelable task @@ -47,12 +49,15 @@ type task struct { } // NewIdler creates a new instance of Idler. The configuration as well as feature toggle handler needs to be passed. -func NewIdler(features toggles.Features, tenantService tenant.Service, clusterView cluster.View, config configuration.Configuration) *Idler { +func NewIdler(features toggles.Features, tenantService tenant.Service, clusterView cluster.View, + config configuration.Configuration) *Idler { return &Idler{ featureService: features, tenantService: tenantService, clusterView: clusterView, config: config, + disabledUsers: model.NewStringSet(), + userIdlers: openshift.NewUserIdlerMap(), } } @@ -73,16 +78,17 @@ func (idler *Idler) Run() { func (idler *Idler) startWorkers(t *task, addProfiler bool) { idlerLogger.Info("Starting all Idler workers") - // Create synchronized map for UserIdler instances - userIdlers := openshift.NewUserIdlerMap() - // Start the controllers to monitor the OpenShift clusters - idler.watchOpenshiftEvents(t, userIdlers) + idler.watchOpenshiftEvents(t) // Start API router go func() { // Create and start a Router instance to serve the REST API - idlerAPI := api.NewIdlerAPI(userIdlers, idler.clusterView, idler.tenantService) + idlerAPI := api.NewIdlerAPI( + idler.userIdlers, + idler.clusterView, + idler.tenantService, + idler.disabledUsers) apirouter := router.CreateAPIRouter(idlerAPI) router := router.NewRouter(apirouter) router.AddMetrics(apirouter) @@ -98,7 +104,7 @@ func (idler *Idler) startWorkers(t *task, addProfiler bool) { } } -func (idler *Idler) watchOpenshiftEvents(t *task, userIdlers *openshift.UserIdlerMap) { +func (idler *Idler) watchOpenshiftEvents(t *task) { oc := client.NewOpenShift() for _, c := range idler.clusterView.GetClusters() { @@ -107,12 +113,13 @@ func (idler *Idler) watchOpenshiftEvents(t *task, userIdlers *openshift.UserIdle t.ctx, c.APIURL, c.Token, - userIdlers, + idler.userIdlers, idler.tenantService, idler.featureService, idler.config, t.wg, t.cancel, + idler.disabledUsers, ) t.wg.Add(2) diff --git a/internal/api/api.go b/internal/api/api.go index eefd888..4cec093 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -14,6 +14,7 @@ import ( "github.com/fabric8-services/fabric8-jenkins-idler/internal/openshift" "github.com/fabric8-services/fabric8-jenkins-idler/internal/openshift/client" "github.com/fabric8-services/fabric8-jenkins-idler/internal/tenant" + "github.com/fabric8-services/fabric8-jenkins-idler/metric" "github.com/julienschmidt/httprouter" log "github.com/sirupsen/logrus" @@ -51,30 +52,42 @@ type IdlerAPI interface { // If an error occurs a response with the HTTP status 400 or 500 is returned. Status(w http.ResponseWriter, r *http.Request, ps httprouter.Params) - // Info writes a JSON representation of internal state of the specified namespace to the response writer. - Info(w http.ResponseWriter, r *http.Request, ps httprouter.Params) - // ClusterDNSView writes a JSON representation of the current cluster state to the response writer. ClusterDNSView(w http.ResponseWriter, r *http.Request, ps httprouter.Params) // Reset deletes a pod and starts a new one Reset(w http.ResponseWriter, r *http.Request, ps httprouter.Params) + + // SetUserIdlerStatus set users status for idler. + SetUserIdlerStatus(w http.ResponseWriter, r *http.Request, ps httprouter.Params) + + // GetDisabledUserIdlers gets the user status for idler. + GetDisabledUserIdlers(w http.ResponseWriter, r *http.Request, ps httprouter.Params) } type idler struct { userIdlers *openshift.UserIdlerMap clusterView cluster.View openShiftClient client.OpenShiftClient - controller openshift.Controller tenantService tenant.Service + disabledUsers *model.StringSet } type status struct { IsIdle bool `json:"is_idle"` } +type userStatus struct { + Disable []string `json:"disable"` + Enable []string `json:"enable"` +} + // NewIdlerAPI creates a new instance of IdlerAPI. -func NewIdlerAPI(userIdlers *openshift.UserIdlerMap, clusterView cluster.View, ts tenant.Service) IdlerAPI { +func NewIdlerAPI( + userIdlers *openshift.UserIdlerMap, + clusterView cluster.View, + ts tenant.Service, + du *model.StringSet) IdlerAPI { // Initialize metrics Recorder.Initialize() return &idler{ @@ -82,6 +95,7 @@ func NewIdlerAPI(userIdlers *openshift.UserIdlerMap, clusterView cluster.View, t clusterView: clusterView, openShiftClient: client.NewOpenShift(), tenantService: ts, + disabledUsers: du, } } @@ -208,17 +222,6 @@ func (api *idler) Status(w http.ResponseWriter, r *http.Request, ps httprouter.P writeResponse(w, http.StatusOK, *response) } -func (api *idler) Info(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - namespace := ps.ByName("namespace") - - userIdler, ok := api.userIdlers.Load(namespace) - if ok { - writeResponse(w, http.StatusOK, userIdler.GetUser()) - } else { - respondWithError(w, http.StatusNotFound, fmt.Errorf("Could not find queried namespace")) - } -} - func (api *idler) ClusterDNSView(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { writeResponse(w, http.StatusOK, api.clusterView.GetDNSView()) } @@ -246,6 +249,30 @@ func (api *idler) Reset(w http.ResponseWriter, r *http.Request, ps httprouter.Pa w.WriteHeader(http.StatusOK) } +//SetUserIdlerStatus sets the user status +func (api *idler) SetUserIdlerStatus(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { + var users userStatus + if err := json.NewDecoder(r.Body).Decode(&users); err != nil { + respondWithError(w, http.StatusBadRequest, err) + return + } + + // enabled users will take precedence over disabled + api.disabledUsers.Add(users.Disable) + api.disabledUsers.Remove(users.Enable) + w.WriteHeader(http.StatusOK) +} + +type idlerStatusResponse struct { + Users []string `json:"users,omitempty"` +} + +//GetDisabledUserIdlers set the user status +func (api *idler) GetDisabledUserIdlers(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { + users := &idlerStatusResponse{Users: api.disabledUsers.Keys()} + writeResponse(w, http.StatusOK, users) +} + func (api *idler) getURLAndToken(r *http.Request) (string, string, error) { var openShiftAPIURL string values, ok := r.URL.Query()[OpenShiftAPIParam] diff --git a/internal/model/stringset.go b/internal/model/stringset.go new file mode 100644 index 0000000..ba0065c --- /dev/null +++ b/internal/model/stringset.go @@ -0,0 +1,29 @@ +package model + +import ( + cmap "github.com/orcaman/concurrent-map" +) + +// StringSet is a type-safe and concurrent map keeping track of disabled v for idler. +type StringSet struct { + cmap.ConcurrentMap +} + +// NewStringSet creates a new instance of StringSet map. +func NewStringSet() *StringSet { + return &StringSet{cmap.New()} +} + +// Add stores the specified value under the key user. +func (m *StringSet) Add(vs []string) { + for _, v := range vs { + m.ConcurrentMap.Set(v, true) + } +} + +// Remove deletes the specified disabled user from the map. +func (m *StringSet) Remove(vs []string) { + for _, v := range vs { + m.ConcurrentMap.Remove(v) + } +} diff --git a/internal/model/stringset_test.go b/internal/model/stringset_test.go new file mode 100644 index 0000000..178a734 --- /dev/null +++ b/internal/model/stringset_test.go @@ -0,0 +1,61 @@ +package model + +import ( + "sort" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestStringSet_new(t *testing.T) { + s := NewStringSet() + assert.NotNil(t, s, "must return a new object") +} + +func TestStringSet_count(t *testing.T) { + s := NewStringSet() + assert.Equal(t, 0, s.Count(), "must have 0 items") +} + +func TestStringSet_add(t *testing.T) { + s := NewStringSet() + s.Add([]string{"foo", "bar"}) + assert.Equal(t, 2, s.Count()) +} + +func TestStringSet_has(t *testing.T) { + s := NewStringSet() + assert.False(t, s.Has("foo"), "must be empty") + s.Add([]string{"foo", "bar"}) + + assert.True(t, s.Has("foo"), "must be present after adding") + assert.True(t, s.Has("bar"), "must be present after adding") + assert.False(t, s.Has("foobar"), "non existent key seems to be found") +} + +func TestStringSet_remove(t *testing.T) { + s := NewStringSet() + assert.Equal(t, 0, s.Count()) + + s.Add([]string{"foo", "bar"}) + assert.Equal(t, 2, s.Count()) + + s.Remove([]string{"bar"}) + assert.Equal(t, 1, s.Count()) + + assert.False(t, s.Has("bar"), "key that got removed still exists") +} + +func TestStringSet_keys(t *testing.T) { + s := NewStringSet() + + keys := []string{"foo", "bar"} + + s.Add(keys) + + values := s.Keys() + sort.Strings(keys) + sort.Strings(values) + + assert.Equal(t, keys, values) +} diff --git a/internal/openshift/controller.go b/internal/openshift/controller.go index 7a3c7b4..71d4841 100644 --- a/internal/openshift/controller.go +++ b/internal/openshift/controller.go @@ -17,6 +17,9 @@ import ( "github.com/sirupsen/logrus" ) +// Status of Users +type Status bool + const ( availableCond = "Available" channelSendTimeout = 1 @@ -44,6 +47,7 @@ type controllerImpl struct { ctx context.Context cancel context.CancelFunc unknownUsers *UnknownUsersMap + disabledUsers *model.StringSet } // NewController creates an instance of controllerImpl. @@ -54,7 +58,9 @@ func NewController( t tenant.Service, features toggles.Features, config configuration.Configuration, - wg *sync.WaitGroup, cancel context.CancelFunc) Controller { + wg *sync.WaitGroup, + cancel context.CancelFunc, + disabledUsers *model.StringSet) Controller { logger.WithField("cluster", openshiftURL).Info("Creating new controller instance") @@ -69,6 +75,7 @@ func NewController( ctx: ctx, cancel: cancel, unknownUsers: NewUnknownUsersMap(), + disabledUsers: disabledUsers, } return &controller @@ -85,7 +92,6 @@ func (c *controllerImpl) HandleBuild(o model.Object) error { "event": "build", "openshift": c.openshiftURL, }) - ok, err := c.createIfNotExist(ns) if err != nil { log.Errorf("Creating user-idler record failed: %s", err) @@ -104,6 +110,10 @@ func (c *controllerImpl) HandleBuild(o model.Object) error { "name": user.Name, }) + if c.disabledUsers.Has(user.Name) { + log.Infof("Status disabled for user: %s", user.Name) + return nil + } evalConditions := false if c.isActive(&o.Object) { @@ -174,6 +184,11 @@ func (c *controllerImpl) HandleDeploymentConfig(dc model.DCObject) error { userIdler := c.userIdlerForNamespace(ns) user := userIdler.GetUser() + if c.disabledUsers.Has(user.Name) { + log.Infof("Status disabled for user: %s", user.Name) + return nil + } + log = log.WithFields(logrus.Fields{ "id": user.ID, "name": user.Name, diff --git a/internal/openshift/controller_test.go b/internal/openshift/controller_test.go index 3d0168c..0a6c2c2 100644 --- a/internal/openshift/controller_test.go +++ b/internal/openshift/controller_test.go @@ -219,7 +219,8 @@ func setUp(t *testing.T) { defer cancel() userIdlers := NewUserIdlerMap() - controller = NewController(ctx, "", "", userIdlers, tenantService, features, &mock.Config{}, &wg, cancel) + disabledUsers := model.NewStringSet() + controller = NewController(ctx, "", "", userIdlers, tenantService, features, &mock.Config{}, &wg, cancel, disabledUsers) } func emptyChannel(ch chan model.User) { diff --git a/internal/router/router.go b/internal/router/router.go index d7e3998..9caa730 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -86,9 +86,6 @@ func (r *Router) Shutdown() { func CreateAPIRouter(api api.IdlerAPI) *httprouter.Router { router := httprouter.New() - router.GET("/api/idler/info/:namespace", api.Info) - router.GET("/api/idler/info/:namespace/", api.Info) - router.GET("/api/idler/idle/:namespace", api.Idle) router.GET("/api/idler/idle/:namespace/", api.Idle) @@ -107,5 +104,11 @@ func CreateAPIRouter(api api.IdlerAPI) *httprouter.Router { router.POST("/api/idler/reset/:namespace", api.Reset) router.POST("/api/idler/reset/:namespace/", api.Reset) + router.GET("/api/idler/userstatus", api.GetDisabledUserIdlers) + router.GET("/api/idler/userstatus/", api.GetDisabledUserIdlers) + + router.POST("/api/idler/userstatus", api.SetUserIdlerStatus) + router.POST("/api/idler/userstatus/", api.SetUserIdlerStatus) + return router } diff --git a/internal/router/router_test.go b/internal/router/router_test.go index a332846..b94b3c5 100644 --- a/internal/router/router_test.go +++ b/internal/router/router_test.go @@ -36,8 +36,6 @@ func Test_all_routes_are_setup(t *testing.T) { route string target string }{ - {"/api/idler/info/my-namepace", "Info"}, - {"/api/idler/info/my-namepace/", "Info"}, {"/api/idler/idle/my-namepace", "Idle"}, {"/api/idler/idle/my-namepace/", "Idle"}, {"/api/idler/unidle/my-namepace", "UnIdle"}, @@ -46,6 +44,10 @@ func Test_all_routes_are_setup(t *testing.T) { {"/api/idler/isidle/my-namepace/", "IsIdle"}, {"/api/idler/cluster", "GetClusterDNSView"}, {"/api/idler/cluster/", "GetClusterDNSView"}, + {"/api/idler/userstatus", "SetUserIdlerStatus"}, + {"/api/idler/userstatus/", "SetUserIdlerStatus"}, + {"/api/idler/userstatus", "GetDisabledUserIdlers"}, + {"/api/idler/userstatus/", "GetDisabledUserIdlers"}, {"/api/idler/foo", "404 page not found\n"}, {"/api/idler/builds/foo/bar", "404 page not found\n"}, @@ -53,11 +55,17 @@ func Test_all_routes_are_setup(t *testing.T) { for _, testRoute := range routes { w := new(mock.ResponseWriter) + if testRoute.target == "SetUserIdlerStatus" { + req, _ := http.NewRequest("POST", testRoute.route, nil) + router.ServeHTTP(w, req) - req, _ := http.NewRequest("GET", testRoute.route, nil) - router.ServeHTTP(w, req) + assert.Equal(t, testRoute.target, w.GetBody(), fmt.Sprintf("Routing failed for %s", testRoute.route)) + } else { + req, _ := http.NewRequest("GET", testRoute.route, nil) + router.ServeHTTP(w, req) - assert.Equal(t, testRoute.target, w.GetBody(), fmt.Sprintf("Routing failed for %s", testRoute.route)) + assert.Equal(t, testRoute.target, w.GetBody(), fmt.Sprintf("Routing failed for %s", testRoute.route)) + } } } @@ -80,10 +88,10 @@ func Test_router_start(t *testing.T) { // we need to give a bit time for the server to come up time.Sleep(1 * time.Second) - resp, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/api/idler/info/foo", testPort)) - assert.NoError(t, err, "The call to the API should have succeeded.") - body, err := ioutil.ReadAll(resp.Body) - assert.Equal(t, "Info", string(body), "Unexpected result from HTTP request") + //resp, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/api/idler/info/foo", testPort)) + //assert.NoError(t, err, "The call to the API should have succeeded.") + //body, err := ioutil.ReadAll(resp.Body) + //assert.Equal(t, "Info", string(body), "Unexpected result from HTTP request") go func() { // Cancel the operation after 2 second. @@ -111,7 +119,7 @@ func Test_cluster_dns_view(t *testing.T) { tenantService, cleanup := stubTenantService() defer cleanup() - idlerAPI := api.NewIdlerAPI(openshift.NewUserIdlerMap(), clusterView, tenantService) + idlerAPI := api.NewIdlerAPI(openshift.NewUserIdlerMap(), clusterView, tenantService, model.NewStringSet()) router := NewRouterWithPort(CreateAPIRouter(idlerAPI), testPort) var wg sync.WaitGroup @@ -160,7 +168,8 @@ func Test_openshift_url_parameter_is_used(t *testing.T) { } clusterView := cluster.NewView([]cluster.Cluster{dummyCluster}) - idlerAPI := api.NewIdlerAPI(openshift.NewUserIdlerMap(), clusterView, tenantService) + + idlerAPI := api.NewIdlerAPI(openshift.NewUserIdlerMap(), clusterView, tenantService, model.NewStringSet()) router := NewRouterWithPort(CreateAPIRouter(idlerAPI), testPort) // start the router diff --git a/internal/testutils/mock/mock_idler_api.go b/internal/testutils/mock/mock_idler_api.go index 4ed0ecf..c57f715 100644 --- a/internal/testutils/mock/mock_idler_api.go +++ b/internal/testutils/mock/mock_idler_api.go @@ -4,18 +4,13 @@ import ( "net/http" "github.com/julienschmidt/httprouter" + log "github.com/sirupsen/logrus" ) // IdlerAPI defines the REST endpoints of the Idler type IdlerAPI struct { } -// Info writes a JSON representation of internal state of the specified namespace to the response writer. -func (i *IdlerAPI) Info(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - w.Write([]byte("Info")) - w.WriteHeader(http.StatusOK) -} - // Idle triggers an idling of the Jenkins service running in the namespace specified in the namespace // parameter of the request. A status code of 200 indicates success whereas 500 indicates failure. func (i *IdlerAPI) Idle(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { @@ -58,3 +53,20 @@ func (i *IdlerAPI) ClusterDNSView(w http.ResponseWriter, req *http.Request, ps h w.Write([]byte("GetClusterDNSView")) w.WriteHeader(http.StatusOK) } + +//SetUserIdlerStatus sets the user status +func (i *IdlerAPI) SetUserIdlerStatus(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { + _, err := w.Write([]byte("SetUserIdlerStatus")) + if err != nil { + log.Error(err) + } + w.WriteHeader(http.StatusOK) +} + +//GetDisabledUserIdlers set the user status +func (i *IdlerAPI) GetDisabledUserIdlers(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { + if _, err := w.Write([]byte("GetDisabledUserIdlers")); err != nil { + log.Error(err) + } + w.WriteHeader(http.StatusOK) +}