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

Add authorization checking to Kubernetes package #2207

Merged
merged 12 commits into from
Sep 10, 2018

Conversation

ebaron
Copy link
Contributor

@ebaron ebaron commented Aug 2, 2018

We are currently working on adding API that determines whether a user is authorized to call various WIT APIs. The goal of this is for the front-end to be able to selectively disable parts of the UI that the user cannot use successfully, rather than enabling them and showing the user an error when used.

This PR implements a portion of this API dealing with the Deployments API, in particular dealing with OpenShift permissions. Using the OpenShift SelfSubjectRulesReview API, we obtain a comprehensive list of actions that the user can perform [1]. This list is processed and cached for fast lookups, for the lifetime of a WIT/Deployments REST API call. There is a new interface which augments the kubernetes.KubeClientInterface with methods that answer whether KubeClientInterface methods can succeed (e.g. KubeAccessControl.CanGetSpace for KubeClientInterface.GetSpace).

When fitting into the larger access control API, we will have to decide how fine-grained we want these checks to be. Do we want to check whether the user can scale deployment X in namespace Y, or simply require that the user be able to scale all deployments in namespace Y. The latter is true for single-user cases, but a cluster admin could specify more fine-grained permissions.

Related issue: #2268

[1] https://docs.openshift.org/3.10/rest_api/oapi/v1.SelfSubjectRulesReview.html

@codecov-io
Copy link

codecov-io commented Aug 2, 2018

Codecov Report

Merging #2207 into master will increase coverage by 0.13%.
The diff coverage is 83.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2207      +/-   ##
==========================================
+ Coverage   69.38%   69.51%   +0.13%     
==========================================
  Files         175      176       +1     
  Lines       16484    16670     +186     
==========================================
+ Hits        11437    11588     +151     
- Misses       3938     3962      +24     
- Partials     1109     1120      +11
Impacted Files Coverage Δ
controller/deployments_urlprovider.go 47.12% <100%> (-0.61%) ⬇️
kubernetes/deployments_kubeclient.go 72.45% <65.38%> (-0.4%) ⬇️
kubernetes/deployments_access.go 88.23% <88.23%> (ø)
remoteworkitem/jira.go 75% <0%> (-25%) ⬇️
remoteworkitem/scheduler.go 53.65% <0%> (-7.32%) ⬇️
controller/workitem.go 78.83% <0%> (+0.56%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2816aa...f596787. Read the comment docs.

@ebaron ebaron force-pushed the deployments-check-k8s-auth branch from d74c13e to b1deea0 Compare August 2, 2018 22:20
Copy link
Contributor

@stooke stooke left a comment

Choose a reason for hiding this comment

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

I think we need to try to look at what OSIO will allow, and that's not yet been defined (to my knowledge).
What makes sense? A shared account might allow individual applications to be shared by different sets people, with varying degrees of access. This is why I suspect we need to tests for "deployment X in namespace Y" - i.e. if we can efficiently get answers we'll need that. Perhaps there are two levels to test - the space level (can the user even read or create an application) and the deployments level (for scaling)?

return true
}

func (kc *kubeClient) CanGetSpace() (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's unclear to me what 'Get' means in "CanGetSpace". Comments and perhaps a more informative name would be great here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My intention here was that if CanGetSpace returns true, then you can successfully call KubeClientInterface.GetSpace. The same is true for the other methods in KubeAccessControl. I can add some documentation for each method explaining this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We may eventually need to know if the user can modify the space or create an application - how would you extend the API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added some documentation for each of the API methods. Checking whether we can create an application and modify a space could be handled in the same fashion, by checking "create" and "patch" permissions respectively on the relevant OpenShift object types.

return false, nil
}

for envName := range kc.envMap {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is why 'Get' is unclear. Does it mean read space, read space and everything in it, can read space and can deploy to it?

return true, nil
}

func (kc *kubeClient) CanGetApplication() (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about 'Get' here.

}

func (kc *kubeClient) checkAuthorizedWithBuilds(envName string, reqs []*requestedAccess) (bool, error) {
// Also need access to builds in user namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a TODO or a comment about what's happening in the current code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a comment pertaining to the current code, but makes less sense after I extracted this to a helper method. I will change it.

@ebaron
Copy link
Contributor Author

ebaron commented Aug 15, 2018

@stooke I hit a bit of a snag with checking permissions of objects with particular names, such as a named deployment config. Our deployments API deals with names of applications. The methods in deployments_kubeclient have to search through Build objects to determine the deployment config name from the application name, which may differ.

In order for our access control API to do the same, it would also have to look at builds to figure out the deployment config name. This would end up evaluating some of the functionality we're trying to determine whether we can invoke in the first place. Perhaps this would be overkill for the purposes of this API.

@stooke
Copy link
Contributor

stooke commented Aug 16, 2018

Maybe we can just leave it at the space level then for now.

env, err := kc.GetEnvironment(envName)
if err != nil {
return nil, err
if kc.CanDeploy(envName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a comment about avoiding getting irrelevant information

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -753,19 +763,19 @@ func (oc *openShiftAPIClient) GetBuildConfigs(namespace string, labelSelector st
return oc.getResource(bcURL, false)
}

func (kc *kubeClient) getEnvironmentNamespace(envName string) (string, error) {
func (kc *kubeClient) getEnvironmentNamespace(envName string, includeInternal bool) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A suggestion: have two methods, one that works as the old one did, and a new one, something like GetDeployableEnvrionmentNamespace(envName), and avoid the bool, which seems to be false most of the time anyways, and just clutters up the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, the boolean parameter made the code less readable.

@stooke stooke mentioned this pull request Aug 17, 2018
@ebaron ebaron changed the title [WIP] Add authorization checking to Kubernetes package Add authorization checking to Kubernetes package Aug 20, 2018
@ebaron ebaron force-pushed the deployments-check-k8s-auth branch 2 times, most recently from fe78458 to 6b37402 Compare August 20, 2018 22:10
@ebaron
Copy link
Contributor Author

ebaron commented Aug 21, 2018

I've added some additional processing for rules containing wildcard group/resources. This matches what is done in the web console.

Copy link
Contributor

@stooke stooke left a comment

Choose a reason for hiding this comment

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

I think this PR is good to go at this point.

@ebaron ebaron force-pushed the deployments-check-k8s-auth branch from 499109d to ad29c04 Compare September 7, 2018 21:14
@ebaron ebaron force-pushed the deployments-check-k8s-auth branch from f69fa6f to f596787 Compare September 10, 2018 19:53
@ebaron ebaron merged commit 426e11b into fabric8-services:master Sep 10, 2018
kwk added a commit to openshiftio/saas-openshiftio that referenced this pull request Sep 17, 2018
# About
This description was generated using this script:
```sh
#!/bin/bash
set -e
GHORG=${GHORG:-fabric8-services}
GHREPO=${GHREPO:-fabric8-wit}
cat <<EOF
# About
This description was generated using this script:
\`\`\`sh
`cat $0`
\`\`\`
Invoked as:

    `echo GHORG=${GHORG} GHREPO=${GHREPO} $(basename $0) ${@:1}`

# Changes
EOF
git log \
  --pretty="%n**Commit:** https://github.com/${GHORG}/${GHREPO}/commit/%H%n**Author:** %an (%ae)%n**Date:** %aI%n%n%s%n%n%b%n%n----%n" \
  --reverse ${@:1} \
  | sed -E "s/([\s|\(| ])#([0-9]+)/\1${GHORG}\/${GHREPO}#\2/g"
```
Invoked as:

    GHORG=fabric8-services GHREPO=fabric8-wit git-log-pr.sh 164762f67a3a7634fa4ee1e8bb55c458281803c7..upstream/master

# Changes

**Commit:** fabric8-services/fabric8-wit@7ab24ac
**Author:** Tina Kurian ([email protected])
**Date:** 2018-09-10T09:47:29-04:00

area iteration root path should not be empty (fabric8-services/fabric8-wit#2214)

See fabric8-services/fabric8-wit#2203

Co-authored-by: Konrad Kleine [email protected]

----


**Commit:** fabric8-services/fabric8-wit@c2816aa
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-09-10T21:47:14+05:30

Update gopkg.lock file (fabric8-services/fabric8-wit#2282)

The `gopkg.lock` file is out of date. This PR updated it.


How to verify this PR
1. Run `dep ensure` on current master. You should see the `gopkg.lock` is modified.
2. Check the changes in `gopkg.lock` file against the ones in this PR. They should be same.
3. Run `dep ensure` again and you would notice `gopkg.lock` has not changed.

----


**Commit:** fabric8-services/fabric8-wit@426e11b
**Author:** Elliott Baron ([email protected])
**Date:** 2018-09-10T16:29:27-04:00

Add authorization checking to Kubernetes package (fabric8-services/fabric8-wit#2207)

* Initial implementation of OpenShift authz checking

* Remove unused interactions in delete cassettes

* Add CanDeploy to filter deployable environments, maintain full list in kubeClient

* Implement authz check for DeleteDeployment

* Implement remaining methods, add tests for access control failure on builds

* Fix URLProvider changes, add tests for CanDeploy

* Better test error conditions

* Clean up code

* Add more internal environments to kubeclient tests

* Improve documentation, avoid boolean parameter for getting namespace

* Also check for wildcard rules

* Reduce size of go-vcr cassettes


----


**Commit:** fabric8-services/fabric8-wit@a8fab53
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-11T08:37:08+02:00

testfixture.GetTestFileAndFunc() (fabric8-services/fabric8-wit#2149)

`GetTestFileAndFunc` returns the file and function of the first `_test.go` file to appear in the stack and returns it in this schema:

```
(see function github.com/fabric8-services/fabric8-wit/test/testfixture_test.TestGetGetTestFileAndFunc in test/testfixture/testfixture_test.go)
```

The result can be used to augment entities so that we always can tell which test created an entity that is a left-over and not cleaned up for example.

NOTE: The `testfixutre.GetTestFileAndFunc()` was taken out of this PR and already merge with this PR fabric8-services/fabric8-wit#2214. It is no longer part of this PR.

Most of the entities created with the `test/testfixture` package now contain a description that reveals in which function the entities was created. When there are left-over entities that hinder tests from running properly, you can now go in the database and find out which test produced these left-overs. To simplify things when running on CI, we now output a list of remaining spaces automatically that looks like this if everything went ok:

```
Check that no unexpected left-over spaces exist after running tests:

-[ RECORD 1 ]-------------------------------------

id          | 2e0698d8-753e-4cef-bb7c-f027634824a2

name        | system.space

description | Description of the space
```

Note, that the `system.space` is okay to appear in this list. It is expected even though it should go away at some point.

See fabric8-services/fabric8-wit#2278 for an example of a PR that was easily fixed because of this PR. 

----


**Commit:** fabric8-services/fabric8-wit@ba71ed1
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-11T12:14:39+02:00

Allow searching for work items with a single quote in the search string (fabric8-services/fabric8-wit#2272)

A query parameter `q` as in `https://prod-preview.openshift.io/api/search?q='problem` was not properly escaped before it was passed to the PostgreSQL `to_tsquery` function. We now remove the single quote from the input to `to_tsquery`.

On the frontend this API was called when search for work items in the "Links" section.

Addresses fabric8-services/fabric8-wit#2273 and openshiftio/openshift.io#4288


----


**Commit:** fabric8-services/fabric8-wit@d6a192d
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-11T12:50:30+02:00

Don't run tests involving the DB in parallel (fabric8-services/fabric8-wit#2276)

Possibly related to openshiftio/openshift.io#4299

----


**Commit:** fabric8-services/fabric8-wit@f276f3a
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-17T14:30:48+02:00

Don't have a zero-parent relationship for root iteration/area (fabric8-services/fabric8-wit#2283)

Before a root iteration/area had a parent iteration with a zeroed UUID:

```yaml
"relationships": {
      "parent": {
        "data": {
          "id": "00000000-0000-0000-0000-000000000000",
          "type": "iterations"
        },
```

That is removed now.

This adds a test that shows how a root and a child iteration/area are represented on JSONAPI level.

Added `testfixture.SetAreaNames()` and `testfixture.AreaByName()` functions as well.

This fixes the root cause for openshiftio/openshift.io#4309 . The UI fix in Raunak1203/fabric8-planner@e7c34a2 only fixes a symptom by implementing a work around.
----


**Commit:** fabric8-services/fabric8-wit@59561d0
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-09-17T18:38:35+05:30

Better error logging for Delete action on /space endpoint (fabric8-services/fabric8-wit#2280)

----
kwk added a commit to openshiftio/saas-openshiftio that referenced this pull request Oct 9, 2018
# About
This description was generated using this script:
```sh
#!/bin/bash
set -e
GHORG=${GHORG:-fabric8-services}
GHREPO=${GHREPO:-fabric8-wit}
cat <<EOF
# About
This description was generated using this script:
\`\`\`sh
`cat $0`
\`\`\`
Invoked as:

    `echo GHORG=${GHORG} GHREPO=${GHREPO} $(basename $0) ${@:1}`

# Changes
EOF
git log \
  --pretty="%n**Commit:** https://github.com/${GHORG}/${GHREPO}/commit/%H%n**Author:** %an (%ae)%n**Date:** %aI%n%n%s%n%n%b%n%n----%n" \
  --reverse ${@:1} \
  | sed -E "s/([\s|\(| ])#([0-9]+)/\1${GHORG}\/${GHREPO}#\2/g"
```
Invoked as:

    GHORG=fabric8-services GHREPO=fabric8-wit git-log-pr.sh 164762f67a3a7634fa4ee1e8bb55c458281803c7..1371e8243fb9f68775a50a8eca738193db706164

# Changes

**Commit:** fabric8-services/fabric8-wit@7ab24ac
**Author:** Tina Kurian ([email protected])
**Date:** 2018-09-10T09:47:29-04:00

area iteration root path should not be empty (fabric8-services/fabric8-wit#2214)

See fabric8-services/fabric8-wit#2203

Co-authored-by: Konrad Kleine [email protected]

----


**Commit:** fabric8-services/fabric8-wit@c2816aa
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-09-10T21:47:14+05:30

Update gopkg.lock file (fabric8-services/fabric8-wit#2282)

The `gopkg.lock` file is out of date. This PR updated it.


How to verify this PR
1. Run `dep ensure` on current master. You should see the `gopkg.lock` is modified.
2. Check the changes in `gopkg.lock` file against the ones in this PR. They should be same.
3. Run `dep ensure` again and you would notice `gopkg.lock` has not changed.

----


**Commit:** fabric8-services/fabric8-wit@426e11b
**Author:** Elliott Baron ([email protected])
**Date:** 2018-09-10T16:29:27-04:00

Add authorization checking to Kubernetes package (fabric8-services/fabric8-wit#2207)

* Initial implementation of OpenShift authz checking

* Remove unused interactions in delete cassettes

* Add CanDeploy to filter deployable environments, maintain full list in kubeClient

* Implement authz check for DeleteDeployment

* Implement remaining methods, add tests for access control failure on builds

* Fix URLProvider changes, add tests for CanDeploy

* Better test error conditions

* Clean up code

* Add more internal environments to kubeclient tests

* Improve documentation, avoid boolean parameter for getting namespace

* Also check for wildcard rules

* Reduce size of go-vcr cassettes


----


**Commit:** fabric8-services/fabric8-wit@a8fab53
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-11T08:37:08+02:00

testfixture.GetTestFileAndFunc() (fabric8-services/fabric8-wit#2149)

`GetTestFileAndFunc` returns the file and function of the first `_test.go` file to appear in the stack and returns it in this schema:

```
(see function github.com/fabric8-services/fabric8-wit/test/testfixture_test.TestGetGetTestFileAndFunc in test/testfixture/testfixture_test.go)
```

The result can be used to augment entities so that we always can tell which test created an entity that is a left-over and not cleaned up for example.

NOTE: The `testfixutre.GetTestFileAndFunc()` was taken out of this PR and already merge with this PR fabric8-services/fabric8-wit#2214. It is no longer part of this PR.

Most of the entities created with the `test/testfixture` package now contain a description that reveals in which function the entities was created. When there are left-over entities that hinder tests from running properly, you can now go in the database and find out which test produced these left-overs. To simplify things when running on CI, we now output a list of remaining spaces automatically that looks like this if everything went ok:

```
Check that no unexpected left-over spaces exist after running tests:

-[ RECORD 1 ]-------------------------------------

id          | 2e0698d8-753e-4cef-bb7c-f027634824a2

name        | system.space

description | Description of the space
```

Note, that the `system.space` is okay to appear in this list. It is expected even though it should go away at some point.

See fabric8-services/fabric8-wit#2278 for an example of a PR that was easily fixed because of this PR. 

----


**Commit:** fabric8-services/fabric8-wit@ba71ed1
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-11T12:14:39+02:00

Allow searching for work items with a single quote in the search string (fabric8-services/fabric8-wit#2272)

A query parameter `q` as in `https://prod-preview.openshift.io/api/search?q='problem` was not properly escaped before it was passed to the PostgreSQL `to_tsquery` function. We now remove the single quote from the input to `to_tsquery`.

On the frontend this API was called when search for work items in the "Links" section.

Addresses fabric8-services/fabric8-wit#2273 and openshiftio/openshift.io#4288


----


**Commit:** fabric8-services/fabric8-wit@d6a192d
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-11T12:50:30+02:00

Don't run tests involving the DB in parallel (fabric8-services/fabric8-wit#2276)

Possibly related to openshiftio/openshift.io#4299

----


**Commit:** fabric8-services/fabric8-wit@f276f3a
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-17T14:30:48+02:00

Don't have a zero-parent relationship for root iteration/area (fabric8-services/fabric8-wit#2283)

Before a root iteration/area had a parent iteration with a zeroed UUID:

```yaml
"relationships": {
      "parent": {
        "data": {
          "id": "00000000-0000-0000-0000-000000000000",
          "type": "iterations"
        },
```

That is removed now.

This adds a test that shows how a root and a child iteration/area are represented on JSONAPI level.

Added `testfixture.SetAreaNames()` and `testfixture.AreaByName()` functions as well.

This fixes the root cause for openshiftio/openshift.io#4309 . The UI fix in Raunak1203/fabric8-planner@e7c34a2 only fixes a symptom by implementing a work around.


----


**Commit:** fabric8-services/fabric8-wit@59561d0
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-09-17T18:38:35+05:30

Better error logging for Delete action on /space endpoint (fabric8-services/fabric8-wit#2280)



----


**Commit:** fabric8-services/fabric8-wit@a917dfe
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-20T23:24:41+02:00

Introduce number column for area and iteration and allow searching it (fabric8-services/fabric8-wit#2287)

This change refactors how a human readable number is assigned to work items. The refactored code makes it easy to have iterations and areas also have a human readable number.

See https://openshift.io/openshiftio/Openshift_io/plan/detail/618

----


**Commit:** fabric8-services/fabric8-wit@71bed56
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T08:48:55+02:00

Empty commit to trigger build (fabric8-services/fabric8-wit#2290)



----


**Commit:** fabric8-services/fabric8-wit@fce4430
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T09:38:26+02:00

Double the amount of allowed time for a pod to be ready (fabric8-services/fabric8-wit#2292)

See fabric8-services/fabric8-wit#2291

----


**Commit:** fabric8-services/fabric8-wit@66831f3
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T18:05:23+02:00

Improve perf of calculating WIs in iteration (fabric8-services/fabric8-wit#2294)

We're seriously wasting time in calculating numbers of work items in
iterations by ignoring the fact that we're in a space. @pbergene gave
me a dump of slow queries this morning and while I was looking for
something completely else I noticed this piece:

```
 "Query Text": "SELECT iterations.id as IterationId, count(*) as Total,\
\    \    \    count( case fields->>'system.state' when 'closed' then '1' else null end ) as Closed FROM \\"work_items\\" left join iterations\
\    \    \    on fields@> concat('{\\"system.iteration\\": \\"', iterations.id, '\\"}')::jsonb WHERE (iterations.space_id = $1\
\    \    \    and work_items.deleted_at IS NULL) GROUP BY IterationId",
```

This query is so slow (~55 to 60ms) that and it appears so often that
the log of slow queries is cluttered with this.

This change is supposed to address this by limiting the number of work
items that are queried to the current space. Also the counting is
simplified by using PostgreSQLs `FILTER` expression (see
https://www.postgresql.org/docs/9.6/static/sql-expressions.html). We have
switched from a `LEFT JOIN` to an `INNER JOIN` for because we can
discard work items in the search result if they don't have an iteration
assigned (thanks to @jarifibrahim for measuring this here: fabric8-services/fabric8-wit#2294 (comment)).

No functionality should have been changed which is why all the test
should continue to work as expected.

----


**Commit:** fabric8-services/fabric8-wit@7fd5f0b
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T18:40:20+02:00

Remove the duration field kind (fabric8-services/fabric8-wit#2289)

We don't have any usage for the duration field kind. No space template uses this field type so far. The duration causes us far more problems than it does good. Especially in making the type system rock-solid, the duration has always caused problems. This is because we tried to store a duration as an `int64` which it really is (see https://godoc.org/time#Duration). The problem with this is that the underlying fields are stored in a JSONB structure. And JSON only supports `float64`. When we assigned an `int64` to `float64` the duration was rounded to the nearest `float64` and this is not accurate.

----


**Commit:** fabric8-services/fabric8-wit@fce0b6a
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T19:12:51+02:00

Double the amount of allowed time for a pod to be ready (fabric8-services/fabric8-wit#2295)

from 60 seconds to now 120 seconds

See fabric8-services/fabric8-wit#2291

----


**Commit:** fabric8-services/fabric8-wit@07f931d
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-09-25T18:38:25+05:30

Improve DeleteSpace error messages (fabric8-services/fabric8-wit#2297)

Minor improvements to errors returned from the Detele action on space controller.

----


**Commit:** fabric8-services/fabric8-wit@cd7a01b
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-25T15:38:13+02:00

10x the amount of allowed time for a pod to be ready (fabric8-services/fabric8-wit#2298)

from 120 seconds to now 1200 seconds

See fabric8-services/fabric8-wit#2291

----


**Commit:** fabric8-services/fabric8-wit@2a95482
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-26T09:38:38+02:00

Use existing number sequences instead of looking them up again (fabric8-services/fabric8-wit#2299)

In order to avoid a sequential table scan on the `work_items` DB table we take the already calculated values for the new `number_sequences` table from the old `work_item_number_sequences`.

Before this was the query plan for `INSERT` into the new `number_sequences` table:

```
EXPLAIN
SELECT space_id, 'work_items' "table_name", MAX(number) 
FROM work_items 
WHERE number IS NOT NULL 
GROUP BY 1,2;
+--------------------------------------------------------------------------------+
| QUERY PLAN                                                                     |
|--------------------------------------------------------------------------------|
| GroupAggregate  (cost=37097.49..38835.71 rows=37629 width=52)                  |
|   Group Key: space_id, 'work_items'::text                                      |
|   ->  Sort  (cost=37097.49..37437.97 rows=136193 width=52)                     |
|         Sort Key: space_id                                                     |
|         ->  Seq Scan on work_items  (cost=0.00..20824.93 rows=136193 width=52) |
|               Filter: (number IS NOT NULL)                                     |
+--------------------------------------------------------------------------------+
```

and now it is:

```
EXPLAIN
SELECT space_id, 'work_items' "table_name", current_val 
FROM work_item_number_sequences
GROUP BY 1,2;
+--------------------------------------------------------------------------------------------------------------------------------+
| QUERY PLAN                                                                                                                     |
|--------------------------------------------------------------------------------------------------------------------------------|
| Group  (cost=0.29..3541.66 rows=43872 width=52)                                                                                |
|   Group Key: space_id, 'work_items'::text                                                                                      |
|   ->  Index Scan using work_item_number_sequences_pkey on work_item_number_sequences  (cost=0.29..3322.30 rows=43872 width=52) |
+--------------------------------------------------------------------------------------------------------------------------------+
```

Thanks go out to @jarifibrahim for bringing my attention to this sequential table scan.

See fabric8-services/fabric8-wit#2291

----


**Commit:** fabric8-services/fabric8-wit@cdef2ae
**Author:** Elliott Baron ([email protected])
**Date:** 2018-09-26T14:43:12-04:00

Add deployments API to compute quota necessary for scale-up (fabric8-services/fabric8-wit#2286)

This PR adds a new API serviced by the deployments controller at /deployments/spaces/{spaceID}/applications/{appName}/deployments/{deployName}/podlimits.

Calling this API on a particular deployment determines the CPU and memory resources required in order to add a new pod to the deployment. This will allow the front-end to decide whether to stop the user from attempting to scale up a deployment if they do not have sufficient quota to do so successfully.

Example usage:
Request: GET https://openshift.io/api/deployments/spaces/$SPACE/applications/$APP/deployments/run/podlimits
Response: {"data":{"limits":{"cpucores":1,"memory":262144000}}}

This work was initially done by @chrislessard, and I have added tests and some modifications.

Fixes: openshiftio/openshift.io#3388

----


**Commit:** fabric8-services/fabric8-wit@8ea84f1
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-28T10:29:51+02:00

Remove work item link category concept (fabric8-services/fabric8-wit#2301)

For over two years we haven't used the link category concept and it is not adding any value except for a theoretical one that can be realized differently directly by modifying a link type; hence I remove this concept.

This involves removal of

- category relationship in the link type's design  (package: `design`)
- category endpoints (packages: `design` and `controller`)
- repository for categories (package: `workitem/link`)
- removal of foreign key constraint on the `link_category_id` column of the `work_item_link_types` table

**NOTE:** the `work_item_link_categories` table or the `link_category_id` column on the `work_item_link_types` table will **NOT** be removed in the change. This is subject of a followup change. The reason is that we want old and new pods to run on the same database. (Thank you to @xcoulon for reminding me of that).

Also a lot of golden files were updated because the sequential part of UUIDs affected a shift in the UUID numbering.

This will fix openshiftio/openshift.io#4299 the hard way :)

----


**Commit:** fabric8-services/fabric8-wit@e483be8
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-28T12:18:06+02:00

acquire exclusive lock on spaces, areas, iterations and work_item_number_sequences table to avoid deadlock during migration (fabric8-services/fabric8-wit#2303)

In order to avoid the following deadlock situation, the 106 migration now acquires an exclusive lock on the `spaces`, `iterations`, `areas` and `work_item_number_sequences` tables.

Legend:
 - relation `36029` = `spaces`
 - relation `36042` = `iterations`

```
Process 875 waits for AccessExclusiveLock on relation 36029 of database 13322; blocked by process 5634.
Process 5634 waits for AccessShareLock on relation 36042 of database 13322; blocked by process 875.
```


----


**Commit:** fabric8-services/fabric8-wit@b80e543
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-01T12:39:11+02:00

Have convert.EqualValue and convert.CascadeEqual (fabric8-services/fabric8-wit#2285)

## About

We need a way to compare if the object stored in the database is the same as the one we have loaded from a space template. In the template we don't care about when an object is created. That's why we need a method that is agnostic to the `gormsupport.Lifecycle` and a `Version` field.

In this change I've extended the `convert.Equaler` interface with a function called `EqualValue` that has the same signature as `Equal`. When implementing `EqualValue` one should focus only on values that make up the object to compare. For example, if an object has a `Version` and a `gormsupport.Lifecycle` member, those are good candidates to be ignored inside of `EqualValue`.

If an object has nested members that themselves implement the `convert.Equaler` interface, we want to call either `Equal` or `EqualValue` on them. It depends on what the outer, containing object is compared with. The `convert.CascadeEqual` takes care of that.

## Minor additional edits

Some implementation of `Equal` tested for the wrong object because they didn't isolate the data properly using subtests. I've fixed that. Also, some implementations of `Equal` didn't test for `Version` or `Lifecycle` differences.

----


**Commit:** fabric8-services/fabric8-wit@54f03e1
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-01T13:19:30+02:00

empty commit to trigger build (fabric8-services/fabric8-wit#2308)



----


**Commit:** fabric8-services/fabric8-wit@8e81220
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-01T14:47:15+02:00

Revert "Introduce number column for area and iteration and allow seahching it (fabric8-services/fabric8-wit#2287)" (fabric8-services/fabric8-wit#2307)

This reverts commit a917dfe49bfeaf5715f9822138b1599e94af00b9 (aka fabric8-services/fabric8-wit#2287).

We're experiencing trouble to migrate the DB for this change (see https://gitlab.cee.redhat.com/dtsd/housekeeping/issues/2349). Currently the prod-preview database is not being updated and stalls at the migration from 105 to 106 (number for iteration an area).

----


**Commit:** fabric8-services/fabric8-wit@af5e62d
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-01T17:10:20+02:00

Add number_sequences table and nullable 'number' column on areas and iterations tables (fabric8-services/fabric8-wit#2309)

This adds back the `number_sequences` table the `number` columns on the `areas` and `iterations` tables (known from fabric8-services/fabric8-wit#2287 but then reverted). But it does this in three individual steps. This change is backwards compatible with the old code as the `number` column is nullable. Except for this structural change, no data is changed.

----


**Commit:** fabric8-services/fabric8-wit@c824993
**Author:** Elliott Baron ([email protected])
**Date:** 2018-10-01T16:43:57-04:00

Add API to show resource usage for the current space (fabric8-services/fabric8-wit#2306)

This PR adds a deployments-related API under /deployments/environments/spaces/:spaceID. This endpoint returns resource usage for each of the user's environments, divided into usage by the specified space and usage by all other spaces combined. Currently this only returns CPU and memory usage, and not objects such as pods or secrets.

There is an older /deployments/spaces/:spaceID/environments endpoint which has been left for backwards compatibility until the front-end is switched over completely.

Example usage:
Request: GET https://openshift.io/api/deployments/environments/spaces/$SPACE_ID
Response:

{
  "data": [
    {
      "attributes": {
        "name": "run",
        "other_usage": {
          "cpucores": {
            "quota": 2,
            "used": 0.488
          },
          "memory": {
            "quota": 1073741824,
            "used": 262144000
          },
          "persistent_volume_claims": {
            "quota": 1,
            "used": 0
          },
          "replication_controllers": {
            "quota": 20,
            "used": 6
          },
          "secrets": {
            "quota": 20,
            "used": 9
          },
          "services": {
            "quota": 5,
            "used": 3
          }
        },
        "space_usage": {
          "cpucores": 1.488,
          "memory": 799014912
        }
      },
      "id": "run",
      "type": "environment"
    }
  ]
}

This work was started by @chrislessard, which I have put the finishing touches on and opened the PR on his behalf.

Fixes: openshiftio/openshift.io#3129

----


**Commit:** fabric8-services/fabric8-wit@e75e0d1
**Author:** Baiju Muthukadan ([email protected])
**Date:** 2018-10-03T17:25:48+05:30

Query language support for child iteration & area (fabric8-services/fabric8-wit#2182)

Query language support to fetch work items that belong to an iteration and its child iterations. Similarly, areas can be used.

Example:-

```
{"iteration": "name", "child": true}
```

The default is true.

----


**Commit:** fabric8-services/fabric8-wit@8795c9f
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-10-08T19:16:45+05:30

Add more logs for Delete action on space controller (fabric8-services/fabric8-wit#2310)

This PR adds more logs to the delete action on space controller.

----


**Commit:** fabric8-services/fabric8-wit@a661437
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-10-09T12:48:37+05:30

Enum.ConvertFromModel should use Basetype.ConvertFromModel method (fabric8-services/fabric8-wit#2224)

The `ConvertFromModel` method on `Enum Type` now uses `ConvertFromModel` method of the `base type` instead of `ConvertToModel`.


----


**Commit:** fabric8-services/fabric8-wit@1371e82
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-09T16:24:11+02:00

drop constraint before modifying the path (fabric8-services/fabric8-wit#2312)

Based on [this discussion](https://chat.openshift.io/developers/pl/6m4cojtiotdoueqot8uww5ybpe) we first remove the unique name index on the `iterations` and `areas` table before we modify the data. 

This should fix the migration issue that was visible here: #1082

----
aslakknutsen pushed a commit to openshiftio/saas-openshiftio that referenced this pull request Oct 9, 2018
# Changes

**Commit:** fabric8-services/fabric8-wit@7ab24ac
**Author:** Tina Kurian ([email protected])
**Date:** 2018-09-10T09:47:29-04:00

area iteration root path should not be empty (fabric8-services/fabric8-wit#2214)

See fabric8-services/fabric8-wit#2203

Co-authored-by: Konrad Kleine [email protected]

----


**Commit:** fabric8-services/fabric8-wit@c2816aa
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-09-10T21:47:14+05:30

Update gopkg.lock file (fabric8-services/fabric8-wit#2282)

The `gopkg.lock` file is out of date. This PR updated it.


How to verify this PR
1. Run `dep ensure` on current master. You should see the `gopkg.lock` is modified.
2. Check the changes in `gopkg.lock` file against the ones in this PR. They should be same.
3. Run `dep ensure` again and you would notice `gopkg.lock` has not changed.

----


**Commit:** fabric8-services/fabric8-wit@426e11b
**Author:** Elliott Baron ([email protected])
**Date:** 2018-09-10T16:29:27-04:00

Add authorization checking to Kubernetes package (fabric8-services/fabric8-wit#2207)

* Initial implementation of OpenShift authz checking

* Remove unused interactions in delete cassettes

* Add CanDeploy to filter deployable environments, maintain full list in kubeClient

* Implement authz check for DeleteDeployment

* Implement remaining methods, add tests for access control failure on builds

* Fix URLProvider changes, add tests for CanDeploy

* Better test error conditions

* Clean up code

* Add more internal environments to kubeclient tests

* Improve documentation, avoid boolean parameter for getting namespace

* Also check for wildcard rules

* Reduce size of go-vcr cassettes


----


**Commit:** fabric8-services/fabric8-wit@a8fab53
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-11T08:37:08+02:00

testfixture.GetTestFileAndFunc() (fabric8-services/fabric8-wit#2149)

`GetTestFileAndFunc` returns the file and function of the first `_test.go` file to appear in the stack and returns it in this schema:

```
(see function github.com/fabric8-services/fabric8-wit/test/testfixture_test.TestGetGetTestFileAndFunc in test/testfixture/testfixture_test.go)
```

The result can be used to augment entities so that we always can tell which test created an entity that is a left-over and not cleaned up for example.

NOTE: The `testfixutre.GetTestFileAndFunc()` was taken out of this PR and already merge with this PR fabric8-services/fabric8-wit#2214. It is no longer part of this PR.

Most of the entities created with the `test/testfixture` package now contain a description that reveals in which function the entities was created. When there are left-over entities that hinder tests from running properly, you can now go in the database and find out which test produced these left-overs. To simplify things when running on CI, we now output a list of remaining spaces automatically that looks like this if everything went ok:

```
Check that no unexpected left-over spaces exist after running tests:

-[ RECORD 1 ]-------------------------------------

id          | 2e0698d8-753e-4cef-bb7c-f027634824a2

name        | system.space

description | Description of the space
```

Note, that the `system.space` is okay to appear in this list. It is expected even though it should go away at some point.

See fabric8-services/fabric8-wit#2278 for an example of a PR that was easily fixed because of this PR. 

----


**Commit:** fabric8-services/fabric8-wit@ba71ed1
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-11T12:14:39+02:00

Allow searching for work items with a single quote in the search string (fabric8-services/fabric8-wit#2272)

A query parameter `q` as in `https://prod-preview.openshift.io/api/search?q='problem` was not properly escaped before it was passed to the PostgreSQL `to_tsquery` function. We now remove the single quote from the input to `to_tsquery`.

On the frontend this API was called when search for work items in the "Links" section.

Addresses fabric8-services/fabric8-wit#2273 and openshiftio/openshift.io#4288


----


**Commit:** fabric8-services/fabric8-wit@d6a192d
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-11T12:50:30+02:00

Don't run tests involving the DB in parallel (fabric8-services/fabric8-wit#2276)

Possibly related to openshiftio/openshift.io#4299

----


**Commit:** fabric8-services/fabric8-wit@f276f3a
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-17T14:30:48+02:00

Don't have a zero-parent relationship for root iteration/area (fabric8-services/fabric8-wit#2283)

Before a root iteration/area had a parent iteration with a zeroed UUID:

```yaml
"relationships": {
      "parent": {
        "data": {
          "id": "00000000-0000-0000-0000-000000000000",
          "type": "iterations"
        },
```

That is removed now.

This adds a test that shows how a root and a child iteration/area are represented on JSONAPI level.

Added `testfixture.SetAreaNames()` and `testfixture.AreaByName()` functions as well.

This fixes the root cause for openshiftio/openshift.io#4309 . The UI fix in Raunak1203/fabric8-planner@e7c34a2 only fixes a symptom by implementing a work around.


----


**Commit:** fabric8-services/fabric8-wit@59561d0
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-09-17T18:38:35+05:30

Better error logging for Delete action on /space endpoint (fabric8-services/fabric8-wit#2280)



----


**Commit:** fabric8-services/fabric8-wit@a917dfe
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-20T23:24:41+02:00

Introduce number column for area and iteration and allow searching it (fabric8-services/fabric8-wit#2287)

This change refactors how a human readable number is assigned to work items. The refactored code makes it easy to have iterations and areas also have a human readable number.

See https://openshift.io/openshiftio/Openshift_io/plan/detail/618

----


**Commit:** fabric8-services/fabric8-wit@71bed56
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T08:48:55+02:00

Empty commit to trigger build (fabric8-services/fabric8-wit#2290)



----


**Commit:** fabric8-services/fabric8-wit@fce4430
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T09:38:26+02:00

Double the amount of allowed time for a pod to be ready (fabric8-services/fabric8-wit#2292)

See fabric8-services/fabric8-wit#2291

----


**Commit:** fabric8-services/fabric8-wit@66831f3
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T18:05:23+02:00

Improve perf of calculating WIs in iteration (fabric8-services/fabric8-wit#2294)

We're seriously wasting time in calculating numbers of work items in
iterations by ignoring the fact that we're in a space. @pbergene gave
me a dump of slow queries this morning and while I was looking for
something completely else I noticed this piece:

```
 "Query Text": "SELECT iterations.id as IterationId, count(*) as Total,\
\    \    \    count( case fields->>'system.state' when 'closed' then '1' else null end ) as Closed FROM \\"work_items\\" left join iterations\
\    \    \    on fields@> concat('{\\"system.iteration\\": \\"', iterations.id, '\\"}')::jsonb WHERE (iterations.space_id = $1\
\    \    \    and work_items.deleted_at IS NULL) GROUP BY IterationId",
```

This query is so slow (~55 to 60ms) that and it appears so often that
the log of slow queries is cluttered with this.

This change is supposed to address this by limiting the number of work
items that are queried to the current space. Also the counting is
simplified by using PostgreSQLs `FILTER` expression (see
https://www.postgresql.org/docs/9.6/static/sql-expressions.html). We have
switched from a `LEFT JOIN` to an `INNER JOIN` for because we can
discard work items in the search result if they don't have an iteration
assigned (thanks to @jarifibrahim for measuring this here: fabric8-services/fabric8-wit#2294 (comment)).

No functionality should have been changed which is why all the test
should continue to work as expected.

----


**Commit:** fabric8-services/fabric8-wit@7fd5f0b
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T18:40:20+02:00

Remove the duration field kind (fabric8-services/fabric8-wit#2289)

We don't have any usage for the duration field kind. No space template uses this field type so far. The duration causes us far more problems than it does good. Especially in making the type system rock-solid, the duration has always caused problems. This is because we tried to store a duration as an `int64` which it really is (see https://godoc.org/time#Duration). The problem with this is that the underlying fields are stored in a JSONB structure. And JSON only supports `float64`. When we assigned an `int64` to `float64` the duration was rounded to the nearest `float64` and this is not accurate.

----


**Commit:** fabric8-services/fabric8-wit@fce0b6a
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-21T19:12:51+02:00

Double the amount of allowed time for a pod to be ready (fabric8-services/fabric8-wit#2295)

from 60 seconds to now 120 seconds

See fabric8-services/fabric8-wit#2291

----


**Commit:** fabric8-services/fabric8-wit@07f931d
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-09-25T18:38:25+05:30

Improve DeleteSpace error messages (fabric8-services/fabric8-wit#2297)

Minor improvements to errors returned from the Detele action on space controller.

----


**Commit:** fabric8-services/fabric8-wit@cd7a01b
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-25T15:38:13+02:00

10x the amount of allowed time for a pod to be ready (fabric8-services/fabric8-wit#2298)

from 120 seconds to now 1200 seconds

See fabric8-services/fabric8-wit#2291

----


**Commit:** fabric8-services/fabric8-wit@2a95482
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-26T09:38:38+02:00

Use existing number sequences instead of looking them up again (fabric8-services/fabric8-wit#2299)

In order to avoid a sequential table scan on the `work_items` DB table we take the already calculated values for the new `number_sequences` table from the old `work_item_number_sequences`.

Before this was the query plan for `INSERT` into the new `number_sequences` table:

```
EXPLAIN
SELECT space_id, 'work_items' "table_name", MAX(number) 
FROM work_items 
WHERE number IS NOT NULL 
GROUP BY 1,2;
+--------------------------------------------------------------------------------+
| QUERY PLAN                                                                     |
|--------------------------------------------------------------------------------|
| GroupAggregate  (cost=37097.49..38835.71 rows=37629 width=52)                  |
|   Group Key: space_id, 'work_items'::text                                      |
|   ->  Sort  (cost=37097.49..37437.97 rows=136193 width=52)                     |
|         Sort Key: space_id                                                     |
|         ->  Seq Scan on work_items  (cost=0.00..20824.93 rows=136193 width=52) |
|               Filter: (number IS NOT NULL)                                     |
+--------------------------------------------------------------------------------+
```

and now it is:

```
EXPLAIN
SELECT space_id, 'work_items' "table_name", current_val 
FROM work_item_number_sequences
GROUP BY 1,2;
+--------------------------------------------------------------------------------------------------------------------------------+
| QUERY PLAN                                                                                                                     |
|--------------------------------------------------------------------------------------------------------------------------------|
| Group  (cost=0.29..3541.66 rows=43872 width=52)                                                                                |
|   Group Key: space_id, 'work_items'::text                                                                                      |
|   ->  Index Scan using work_item_number_sequences_pkey on work_item_number_sequences  (cost=0.29..3322.30 rows=43872 width=52) |
+--------------------------------------------------------------------------------------------------------------------------------+
```

Thanks go out to @jarifibrahim for bringing my attention to this sequential table scan.

See fabric8-services/fabric8-wit#2291

----


**Commit:** fabric8-services/fabric8-wit@cdef2ae
**Author:** Elliott Baron ([email protected])
**Date:** 2018-09-26T14:43:12-04:00

Add deployments API to compute quota necessary for scale-up (fabric8-services/fabric8-wit#2286)

This PR adds a new API serviced by the deployments controller at /deployments/spaces/{spaceID}/applications/{appName}/deployments/{deployName}/podlimits.

Calling this API on a particular deployment determines the CPU and memory resources required in order to add a new pod to the deployment. This will allow the front-end to decide whether to stop the user from attempting to scale up a deployment if they do not have sufficient quota to do so successfully.

Example usage:
Request: GET https://openshift.io/api/deployments/spaces/$SPACE/applications/$APP/deployments/run/podlimits
Response: {"data":{"limits":{"cpucores":1,"memory":262144000}}}

This work was initially done by @chrislessard, and I have added tests and some modifications.

Fixes: openshiftio/openshift.io#3388

----


**Commit:** fabric8-services/fabric8-wit@8ea84f1
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-28T10:29:51+02:00

Remove work item link category concept (fabric8-services/fabric8-wit#2301)

For over two years we haven't used the link category concept and it is not adding any value except for a theoretical one that can be realized differently directly by modifying a link type; hence I remove this concept.

This involves removal of

- category relationship in the link type's design  (package: `design`)
- category endpoints (packages: `design` and `controller`)
- repository for categories (package: `workitem/link`)
- removal of foreign key constraint on the `link_category_id` column of the `work_item_link_types` table

**NOTE:** the `work_item_link_categories` table or the `link_category_id` column on the `work_item_link_types` table will **NOT** be removed in the change. This is subject of a followup change. The reason is that we want old and new pods to run on the same database. (Thank you to @xcoulon for reminding me of that).

Also a lot of golden files were updated because the sequential part of UUIDs affected a shift in the UUID numbering.

This will fix openshiftio/openshift.io#4299 the hard way :)

----


**Commit:** fabric8-services/fabric8-wit@e483be8
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-09-28T12:18:06+02:00

acquire exclusive lock on spaces, areas, iterations and work_item_number_sequences table to avoid deadlock during migration (fabric8-services/fabric8-wit#2303)

In order to avoid the following deadlock situation, the 106 migration now acquires an exclusive lock on the `spaces`, `iterations`, `areas` and `work_item_number_sequences` tables.

Legend:
 - relation `36029` = `spaces`
 - relation `36042` = `iterations`

```
Process 875 waits for AccessExclusiveLock on relation 36029 of database 13322; blocked by process 5634.
Process 5634 waits for AccessShareLock on relation 36042 of database 13322; blocked by process 875.
```


----


**Commit:** fabric8-services/fabric8-wit@b80e543
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-01T12:39:11+02:00

Have convert.EqualValue and convert.CascadeEqual (fabric8-services/fabric8-wit#2285)

## About

We need a way to compare if the object stored in the database is the same as the one we have loaded from a space template. In the template we don't care about when an object is created. That's why we need a method that is agnostic to the `gormsupport.Lifecycle` and a `Version` field.

In this change I've extended the `convert.Equaler` interface with a function called `EqualValue` that has the same signature as `Equal`. When implementing `EqualValue` one should focus only on values that make up the object to compare. For example, if an object has a `Version` and a `gormsupport.Lifecycle` member, those are good candidates to be ignored inside of `EqualValue`.

If an object has nested members that themselves implement the `convert.Equaler` interface, we want to call either `Equal` or `EqualValue` on them. It depends on what the outer, containing object is compared with. The `convert.CascadeEqual` takes care of that.

## Minor additional edits

Some implementation of `Equal` tested for the wrong object because they didn't isolate the data properly using subtests. I've fixed that. Also, some implementations of `Equal` didn't test for `Version` or `Lifecycle` differences.

----


**Commit:** fabric8-services/fabric8-wit@54f03e1
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-01T13:19:30+02:00

empty commit to trigger build (fabric8-services/fabric8-wit#2308)



----


**Commit:** fabric8-services/fabric8-wit@8e81220
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-01T14:47:15+02:00

Revert "Introduce number column for area and iteration and allow seahching it (fabric8-services/fabric8-wit#2287)" (fabric8-services/fabric8-wit#2307)

This reverts commit a917dfe49bfeaf5715f9822138b1599e94af00b9 (aka fabric8-services/fabric8-wit#2287).

We're experiencing trouble to migrate the DB for this change (see https://gitlab.cee.redhat.com/dtsd/housekeeping/issues/2349). Currently the prod-preview database is not being updated and stalls at the migration from 105 to 106 (number for iteration an area).

----


**Commit:** fabric8-services/fabric8-wit@af5e62d
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-01T17:10:20+02:00

Add number_sequences table and nullable 'number' column on areas and iterations tables (fabric8-services/fabric8-wit#2309)

This adds back the `number_sequences` table the `number` columns on the `areas` and `iterations` tables (known from fabric8-services/fabric8-wit#2287 but then reverted). But it does this in three individual steps. This change is backwards compatible with the old code as the `number` column is nullable. Except for this structural change, no data is changed.

----


**Commit:** fabric8-services/fabric8-wit@c824993
**Author:** Elliott Baron ([email protected])
**Date:** 2018-10-01T16:43:57-04:00

Add API to show resource usage for the current space (fabric8-services/fabric8-wit#2306)

This PR adds a deployments-related API under /deployments/environments/spaces/:spaceID. This endpoint returns resource usage for each of the user's environments, divided into usage by the specified space and usage by all other spaces combined. Currently this only returns CPU and memory usage, and not objects such as pods or secrets.

There is an older /deployments/spaces/:spaceID/environments endpoint which has been left for backwards compatibility until the front-end is switched over completely.

Example usage:
Request: GET https://openshift.io/api/deployments/environments/spaces/$SPACE_ID
Response:

{
  "data": [
    {
      "attributes": {
        "name": "run",
        "other_usage": {
          "cpucores": {
            "quota": 2,
            "used": 0.488
          },
          "memory": {
            "quota": 1073741824,
            "used": 262144000
          },
          "persistent_volume_claims": {
            "quota": 1,
            "used": 0
          },
          "replication_controllers": {
            "quota": 20,
            "used": 6
          },
          "secrets": {
            "quota": 20,
            "used": 9
          },
          "services": {
            "quota": 5,
            "used": 3
          }
        },
        "space_usage": {
          "cpucores": 1.488,
          "memory": 799014912
        }
      },
      "id": "run",
      "type": "environment"
    }
  ]
}

This work was started by @chrislessard, which I have put the finishing touches on and opened the PR on his behalf.

Fixes: openshiftio/openshift.io#3129

----


**Commit:** fabric8-services/fabric8-wit@e75e0d1
**Author:** Baiju Muthukadan ([email protected])
**Date:** 2018-10-03T17:25:48+05:30

Query language support for child iteration & area (fabric8-services/fabric8-wit#2182)

Query language support to fetch work items that belong to an iteration and its child iterations. Similarly, areas can be used.

Example:-

```
{"iteration": "name", "child": true}
```

The default is true.

----


**Commit:** fabric8-services/fabric8-wit@8795c9f
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-10-08T19:16:45+05:30

Add more logs for Delete action on space controller (fabric8-services/fabric8-wit#2310)

This PR adds more logs to the delete action on space controller.

----


**Commit:** fabric8-services/fabric8-wit@a661437
**Author:** Ibrahim Jarif ([email protected])
**Date:** 2018-10-09T12:48:37+05:30

Enum.ConvertFromModel should use Basetype.ConvertFromModel method (fabric8-services/fabric8-wit#2224)

The `ConvertFromModel` method on `Enum Type` now uses `ConvertFromModel` method of the `base type` instead of `ConvertToModel`.


----


**Commit:** fabric8-services/fabric8-wit@1371e82
**Author:** Konrad Kleine ([email protected])
**Date:** 2018-10-09T16:24:11+02:00

drop constraint before modifying the path (fabric8-services/fabric8-wit#2312)

Based on [this discussion](https://chat.openshift.io/developers/pl/6m4cojtiotdoueqot8uww5ybpe) we first remove the unique name index on the `iterations` and `areas` table before we modify the data. 

This should fix the migration issue that was visible here: #1082

----
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants