Add role for metrics #19651

svor · 2021-04-22T10:58:53Z

What does this PR do?

Adds new role workspace-metrics for workspace ServiceAccount to get an information about workspace's resources from Metric server. It makes possible to read metrics of the workspace which is running in different namespace than che.

What issues does this PR fix or reference?

#18812

How to test this PR?

To check this changes, you need:

deploy che with custom cheimage with current changes
try to create a workspace and check that it was created in the namespace username-che which is different than the namespes where che was deployed
try to get roles for username-che namespace

As a result workspace-metrics should be existed in the list

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

che-bot · 2021-04-22T11:08:30Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: quay.io/crw_pr/che-server:19651

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

skabashnyuk · 2021-04-22T12:40:52Z

Can you explain how it works? How to test it?

svor · 2021-04-22T13:47:11Z

Hi @skabashnyuk I've updated the description, please let me know if something is unclear

amisevsk

I haven't tested changes but it makes sense to me. To be clear, the added RBAC is still restricted to the workspace's namespace, right?

sleshchenko

The only concern I have with this PR, it does not seem to affect existing users, who have SA already initialized, but I'm happy to be wrong here https://github.com/svor/che/blob/sv-metrics/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java#L113

@metlos I remember you worked in this area and we already discussed how we should update/why we should not update SA and roles, but I don't remember details.

skabashnyuk · 2021-04-26T07:11:15Z

Please merge the latest master to fix the build check.

Signed-off-by: svor <[email protected]>

metlos · 2021-04-27T19:25:28Z

The only concern I have with this PR, it does not seem to affect existing users, who have SA already initialized, but I'm happy to be wrong here https://github.com/svor/che/blob/sv-metrics/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java#L113

@metlos I remember you worked in this area and we already discussed how we should update/why we should not update SA and roles, but I don't remember details.

That is correct. The service account is only created once and not touched afterwards. Maybe we should revisit that and make sure the service account has all the rolebindings we require (and keep any additional that the admin may have added there).

metlos

LGTM, but as @sleshchenko said, we need to follow this up by another issue where we should make sure the service account has bindings to all the roles we require.

skabashnyuk · 2021-04-28T07:28:02Z

LGTM, but as @sleshchenko said, we need to follow this up by another issue where we should make sure the service account has bindings to all the roles we require.

Who can do that(create an issue)?

svor · 2021-04-28T09:01:51Z

@skabashnyuk here is the issue #19697
@metlos could you please add some additional information into the issue if it is required

svor · 2021-04-28T09:05:19Z

[crw-ci-test --rebuild]

Ohrimenko1988 · 2021-04-28T11:17:57Z

[crw-ci-test --rebuild]

che-bot · 2021-04-28T11:44:54Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: quay.io/crw_pr/che-server:19651

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

dmytro-ndp · 2021-04-28T13:10:52Z

@svor: Happy path tests above have failed because test workspace hasn't started:

2021-04-28 11:44:44,764[nio-8080-exec-2]  [INFO ] [o.e.c.a.w.s.WorkspaceManager 694]    - Workspace 'admin/petclinic-dev-environment' with id 'workspace3yh5x9errn6e43b5' created by user 'admin'
2021-04-28 11:44:47,340[nio-8080-exec-3]  [ERROR] [c.a.c.r.RuntimeExceptionMapper 47]   - Internal Server Error occurred, error time: 2021-04-28 11:44:47
io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: POST at: https://10.96.0.1/apis/rbac.authorization.k8s.io/v1/namespaces/admin-che/roles. Message: Forbidden!Configured service account doesn't have access. Service account may have been revoked. roles.rbac.authorization.k8s.io "workspace-metrics" is forbidden: user "system:serviceaccount:eclipse-che:che" (groups=["system:serviceaccounts" "system:serviceaccounts:eclipse-che" "system:authenticated"]) is attempting to grant RBAC permissions not currently held:
{APIGroups:["metrics.k8s.io"], Resources:["nodes"], Verbs:["list" "get" "watch"]}
{APIGroups:["metrics.k8s.io"], Resources:["pods"], Verbs:["list" "get" "watch"]}.
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.requestFailure(OperationSupport.java:568)
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.assertResponseCode(OperationSupport.java:505)
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse(OperationSupport.java:471)
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse(OperationSupport.java:430)
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleCreate(OperationSupport.java:251)
	at io.fabric8.kubernetes.client.dsl.base.BaseOperation.handleCreate(BaseOperation.java:815)
	at io.fabric8.kubernetes.client.dsl.base.BaseOperation.create(BaseOperation.java:333)
	at org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.createRole(AbstractWorkspaceServiceAccount.java:239)
	at org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.createRoleWithBinding(AbstractWorkspaceServiceAccount.java:163)
	at org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.createImplicitRolesWithBindings(AbstractWorkspaceServiceAccount.java:147)
	at org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.prepare(AbstractWorkspaceServiceAccount.java:116)
	at org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory.getOrCreate(KubernetesNamespaceFactory.java:441)
	at org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesRuntimeContext.getRuntime(KubernetesRuntimeContext.java:76)
	at org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesRuntimeContext.getRuntime(KubernetesRuntimeContext.java:33)
	at org.eclipse.che.api.workspace.server.WorkspaceRuntimes.startAsync(WorkspaceRuntimes.java:467)
	at org.eclipse.che.api.workspace.server.WorkspaceManager.startAsync(WorkspaceManager.java:529)
	at org.eclipse.che.api.workspace.server.WorkspaceManager.startWorkspace(WorkspaceManager.java:383)
	at org.eclipse.che.multiuser.resource.api.workspace.LimitsCheckingWorkspaceManager.startWorkspace(LimitsCheckingWorkspaceManager.java:135)
	at org.eclipse.che.api.workspace.server.WorkspaceService.startById(WorkspaceService.java:417)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.base/java.lang.reflect.Method.invoke(Unknown Source)
	at org.everrest.core.impl.method.DefaultMethodInvoker.invokeMethod(DefaultMethodInvoker.java:141)
	at org.everrest.core.impl.method.DefaultMethodInvoker.invokeMethod(DefaultMethodInvoker.java:61)
	at org.everrest.core.impl.RequestDispatcher.doInvokeResource(RequestDispatcher.java:307)
	at org.everrest.core.impl.RequestDispatcher.invokeSubResourceMethod(RequestDispatcher.java:298)
	at org.everrest.core.impl.RequestDispatcher.dispatch(RequestDispatcher.java:234)
	at org.everrest.core.impl.RequestDispatcher.dispatch(RequestDispatcher.java:129)
	at org.everrest.core.impl.RequestHandlerImpl.handleRequest(RequestHandlerImpl.java:63)
	at org.everrest.core.impl.EverrestProcessor.process(EverrestProcessor.java:121)
	at org.everrest.core.servlet.EverrestServlet.service(EverrestServlet.java:62)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:733)
	at com.google.inject.servlet.ServletDefinition.doServiceImpl(ServletDefinition.java:290)
	at com.google.inject.servlet.ServletDefinition.doService(ServletDefinition.java:280)
	at com.google.inject.servlet.ServletDefinition.service(ServletDefinition.java:184)
	at com.google.inject.servlet.ManagedServletPipeline.service(ManagedServletPipeline.java:89)
	at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:85)
	at org.eclipse.che.core.metrics.ApiResponseMetricFilter.doFilter(ApiResponseMetricFilter.java:46)
	at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
	at org.eclipse.che.commons.logback.filter.IdentityIdLoggerFilter.doFilter(IdentityIdLoggerFilter.java:49)
	at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
	at org.eclipse.che.multiuser.api.authentication.commons.filter.MultiUserEnvironmentInitializationFilter.doFilter(MultiUserEnvironmentInitializationFilter.java:142)
	at org.eclipse.che.multiuser.keycloak.server.KeycloakEnvironmentInitializationFilter.doFilter(KeycloakEnvironmentInitializationFilter.java:99)
	at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
	at org.eclipse.che.multiuser.machine.authentication.server.MachineLoginFilter.doFilter(MachineLoginFilter.java:76)
	at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
	at org.eclipse.che.commons.logback.filter.RequestIdLoggerFilter.doFilter(RequestIdLoggerFilter.java:50)
	at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
	at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:121)
	at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:133)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:199)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:97)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:544)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:143)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:81)
	at org.apache.catalina.valves.RemoteIpValve.invoke(RemoteIpValve.java:764)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:78)
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:364)
	at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:616)
	at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
	at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:831)
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1629)
	at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
	at java.base/java.lang.Thread.run(Unknown Source)

svor · 2021-04-29T07:49:34Z

to make happy path tests happy, firstly need to merge eclipse-che/che-operator#793

artaleks9 · 2021-05-05T11:04:13Z

[crw-ci-test --rebuild]

che-bot · 2021-05-05T11:29:55Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: quay.io/crw_pr/che-server:19651

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

svor self-assigned this Apr 22, 2021

svor requested a review from metlos April 22, 2021 10:59

svor marked this pull request as ready for review April 22, 2021 10:59

svor requested review from amisevsk, nickboldt, skabashnyuk and sleshchenko as code owners April 22, 2021 10:59

che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Apr 22, 2021

svor mentioned this pull request Apr 22, 2021

Rework reading pod's information for resource monitor plugin eclipse-che/che-theia#1089

Merged

9 tasks

amisevsk approved these changes Apr 23, 2021

View reviewed changes

sleshchenko reviewed Apr 23, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

Add role for metrics

98a97e3

Signed-off-by: svor <[email protected]>

This comment has been minimized.

Sign in to view

metlos approved these changes Apr 27, 2021

View reviewed changes

svor mentioned this pull request Apr 28, 2021

Update existing ServiceAccount if some Role/RoleBinding was changed #19697

Closed

This comment has been minimized.

Sign in to view

Merge branch 'master' into sv-metrics

0c7d9a1

eclipse-che deleted a comment from che-bot May 5, 2021

This comment has been minimized.

Sign in to view

svor merged commit 997ce18 into eclipse-che:master May 5, 2021

che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label May 5, 2021

che-bot added this to the 7.30 milestone May 5, 2021

skabashnyuk mentioned this pull request Jul 13, 2021

Workspace namespace is not prepared properly if metrics.k8s.io API is not enabled on the cluster #19869

Closed

23 tasks

mshaposhnik mentioned this pull request Sep 2, 2021

Che SA may not always have permissions to create metrics.k8s.io roles #20396

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add role for metrics #19651

Add role for metrics #19651

svor commented Apr 22, 2021 •

edited

Loading

che-bot commented Apr 22, 2021

skabashnyuk commented Apr 22, 2021

svor commented Apr 22, 2021

amisevsk left a comment

sleshchenko left a comment

skabashnyuk commented Apr 26, 2021

This comment has been minimized.

This comment has been minimized.

metlos commented Apr 27, 2021 •

edited

Loading

metlos left a comment

skabashnyuk commented Apr 28, 2021

svor commented Apr 28, 2021

svor commented Apr 28, 2021

This comment has been minimized.

Ohrimenko1988 commented Apr 28, 2021

che-bot commented Apr 28, 2021

dmytro-ndp commented Apr 28, 2021

svor commented Apr 29, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

artaleks9 commented May 5, 2021

che-bot commented May 5, 2021

Add role for metrics #19651

Add role for metrics #19651

Conversation

svor commented Apr 22, 2021 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

che-bot commented Apr 22, 2021

skabashnyuk commented Apr 22, 2021

svor commented Apr 22, 2021

amisevsk left a comment

Choose a reason for hiding this comment

sleshchenko left a comment

Choose a reason for hiding this comment

skabashnyuk commented Apr 26, 2021

This comment has been minimized.

This comment has been minimized.

metlos commented Apr 27, 2021 • edited Loading

metlos left a comment

Choose a reason for hiding this comment

skabashnyuk commented Apr 28, 2021

svor commented Apr 28, 2021

svor commented Apr 28, 2021

This comment has been minimized.

Ohrimenko1988 commented Apr 28, 2021

che-bot commented Apr 28, 2021

dmytro-ndp commented Apr 28, 2021

svor commented Apr 29, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

artaleks9 commented May 5, 2021

che-bot commented May 5, 2021

svor commented Apr 22, 2021 •

edited

Loading

metlos commented Apr 27, 2021 •

edited

Loading