Delete space does not delete che workspace

[ljelinkova]

Issue Overview

When you have space in OSIO with created Che workspace, neither delete space action neither reset of environment action deletes the Che workspace.

Expected Behaviour

The Che workspace should be deleted when associated space is deleted.

Current Behaviour

The Che workspace is NOT deleted when associated space is deleted.

Steps To Reproduce

1. Create space, application and workspace in OSIO
2. Check that workspace is listed in Che dashboard che.openshift.io
3. Delete space OR reset environment
4. Check that workspace is still present in Che dashboard

Additional Information

Note that this causes E2E tests fail because there are too many workspaces which causes some Gateway timeout errors when running Che.

I've found the following code in DeleteSpace method

log.Info(ctx, nil, "Found %d workspaces to delete", len(workspaces))
	for _, workspace := range workspaces {
		for _, link := range workspace.Links {
			if strings.ToLower(link.Method) == "delete" {
				log.Info(ctx,
					map[string]interface{}{"codebase_url": cb.URL,
						"che_namespace": ns,
						"workspace":     workspace.Config.Name,
					}, "About to delete Che workspace")

@rhopp checked Kibana logs and it seems that the first log is logged but not the second one. This makes me believe that the returned workspace somehow does not contain any link or the returned link does not contain 'delete' method.

@xcoulon, @ibuziuk FYI

[rhopp]

I guess, that WIT uses <che-api-url>/workspace for obtaining all workspaces... This endpoint always returns empty links section, even though querying <che-api-url>/workspace/<workspace_id> shows links section populated (with self, ide, .. links). Possible bug in upstream che?

Either way, I think the logic in WIT should be changed to not depend on links, but instead construct the correct URLs by itself (<che-api-url>/workspace/<workspace_id>)

[xcoulon]

I guess, that WIT uses <che-api-url>/workspace for obtaining all workspaces... This endpoint always returns empty links section, even though querying <che-api-url>/workspace/<workspace_id> shows links section populated (with self, ide, .. links). Possible bug in upstream che?

Either way, I think the logic in WIT should be changed to not depend on links, but instead construct the correct URLs by itself (<che-api-url>/workspace/<workspace_id>)

On the contrary, I believe that Che should return valid links that WIT can follow. The client service should not "build" URLs itself, but rather, it should rely on a previous response from the remote service.

[rhopp]

I guess, that WIT uses <che-api-url>/workspace for obtaining all workspaces... This endpoint always returns empty links section, even though querying <che-api-url>/workspace/<workspace_id> shows links section populated (with self, ide, .. links). Possible bug in upstream che?
Either way, I think the logic in WIT should be changed to not depend on links, but instead construct the correct URLs by itself (<che-api-url>/workspace/<workspace_id>)

On the contrary, I believe that Che should return valid links that WIT can follow. The client service should not "build" URLs itself, but rather, it should rely on a previous response from the remote service.

Yep... I was writing faster than thinking :-D You are 100% correct...

[xcoulon]

I guess, that WIT uses <che-api-url>/workspace for obtaining all workspaces... This endpoint always returns empty links section, even though querying <che-api-url>/workspace/<workspace_id> shows links section populated (with self, ide, .. links). Possible bug in upstream che?
Either way, I think the logic in WIT should be changed to not depend on links, but instead construct the correct URLs by itself (<che-api-url>/workspace/<workspace_id>)

On the contrary, I believe that Che should return valid links that WIT can follow. The client service should not "build" URLs itself, but rather, it should rely on a previous response from the remote service.

Yep... I was writing faster than thinking :-D You are 100% correct...

No worries, @rhopp ;)
@ibuziuk is it something you could take a look at, please?

[ibuziuk]

Note that this causes E2E tests fail because there are too many workspaces which causes some Gateway timeout errors when running Che.

@ljelinkova to unblock e2e tests could all the previous workspaces been removed ? also why after test execution cleanup of previously created workspaces does not happen ?

[ibuziuk]

@xcoulon che-starter already provides API for deleting workspace by name:

DELETE /workspace/{name}

hmm... I do not really understand why it is currently expected that delete link exists at all, since it has never been part of response from che-starter. I believe we should simply remove delete link check [1] and simply execute removal of the workspace by name [2] in the loop. It is quite strange to see delete link checks which are actually never used for workspace deletion. Does it make sense ?

Either way, I think the logic in WIT should be changed to not depend on links, but instead construct the correct URLs by itself (/workspace/<workspace_id>)

Wit should never call che-server API directly, all communication should happen via che-starter that should play the role of API adapter

[1] https://github.com/fabric8-services/fabric8-wit/blob/master/controller/codebase.go#L257
[2] https://github.com/fabric8-services/fabric8-wit/blob/master/controller/codebase.go#L263

[ljelinkova]

@ibuziuk Thanks for the answer. We've already taken care of e2e by removing all old workspaces. We do not automatically delete old workspaces because that is something normal user should not need to do. We wouldn't even find this problem if we were deleting them automatically.

@xcoulon How do you propose to proceed? Will you rewrite the WIT to use che-starter delete workspace API?

[ppitonak]

@xcoulon any updates?

[ppitonak]

@xcoulon when is this planned to be fixed? It causes trouble in production. We manually remove old che workspaces in our testing accounts but cannot expect our customers to do the same.

[alexeykazakov]

@ppitonak it's not for @xcoulon (Core team) to answer that question :)
It's for WIT team to fix that (cc: @sbose78 )

[sbose78]

Thanks @alexeykazakov .

[ppitonak]

We need to periodically remove che workspaces on our testing accounts, otherwise the product is unusable eclipse-che/che#12638

[jarifibrahim]

I've verified that this issue is fixed on prod-preview. @ppitonak can you confirm? I'll push the changes to prod once we're sure the issue is fixed.

[ppitonak]

Works for me, thanks!