Problem/Motivation

  • Get K8s Pod's log by REST API

Proposed resolution

  • Add a REST API to the cloud module to get K8s Pod's log
CommentFileSizeAuthor
#6 3227081-2.patch3.43 KBryo yamashita

Issue fork cloud-3227081

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Ryo Yamashita created an issue. See original summary.

Ryo Yamashita opened merge request !338

ryo yamashita’s picture

ryo yamashita commented
Status: Needs work » Needs review
ryo yamashita’s picture

ryo yamashita commented

@yas

I have created a patch. Could you please check it?

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@RyoYamashita

Thank you for supplying the patch. I posted the comment for refactoring. Thanks!

ryo yamashita’s picture

ryo yamashita commented
StatusFileSize
new 3227081-2.patch3.43 KB

@yas

I checked your comment.
I modified the code as you pointed out and got an error like "Cannot instantiate interface Drupal\k8sService\K8sRestServiceInterface".
Please refer to the following log text and patch file for specific confirmation.

Error: Drupal\Component\DependencyInjection\Container->createService() 内 Cannot instantiate interface Drupal\k8s\Service\K8sRestServiceInterface (/var/www/html/web/core/lib/Drupal/Component/DependencyInjection/Container.php の 262 行目)
#0 /var/www/html/web/core/lib/Drupal/Component/DependencyInjection/Container.php(176): Drupal\Component\DependencyInjection\Container->createService(Array, 'k8s_rest')
#1 /var/www/html/web/modules/contrib/cloud/modules/cloud_service_providers/k8s/src/Controller/ApiController.php(181): Drupal\Component\DependencyInjection\Container->get('k8s_rest')
#2 /var/www/html/web/core/lib/Drupal/Core/DependencyInjection/ClassResolver.php(28): Drupal\k8s\Controller\ApiController::create(Object(Drupal\Core\DependencyInjection\Container))

(The rest is omitted)

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented

@RyoYamashita

Could you please attach the patch? Definitely there is some problem about instantiation of the service. Thanks!

ryo yamashita’s picture

ryo yamashita commented

@yas

The patch file is 3227081-2.patch.
I thought I could implement the service by referring to K8sService, but I am having trouble with the above error.
I would be grateful for your advice.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented

@RyoYamashita

Sorry I've missed the attached patch since I thought you'd updated MR branch.

Anyhow, I couldn't find the K8sRestServiceInterface definition.

I think we have something like this:

k8s.service.yml

  ...

  k8s.rest.factory:
    class: Drupal\k8s\Service\K8sRestServiceFactory
    arguments: ['@entity_type.manager', '@config.factory', '@current_user', '@plugin.manager.cloud_config_plugin', '@plugin.manager.field.field_type', '@entity_field.manager', '@lock', '@queue', '@module_handler', '@class_resolver']


  k8s.rest:
    factory: ['@k8s.rest.factory', 'create']
    class: Drupal\k8s\Service\K8sServiceInterface
  ...

Then, K8sRestServiceFactory should return a K8sRestService class. To implement K8sRestServiceFactory, see also and refer to the K8sServiceFactory class. The K8sRestService class should implement K8sServiceInterface as I mentioned my comment above. Or, probably we can re-use the existing K8sServiceFactory to get K8sRestService.

@xiaohua-guan

What do you think?

ryo yamashita’s picture

ryo yamashita commented

@yas

Thank you for all the advice you have given me.
It didn't occur to me that I also need to implement the factory class.
The following are the additions to the k8s.service.yml, which do not include the factory class.

services:
  k8s_rest:
    class: Drupal\k8s\Service\K8sRestServiceInterface
    arguments: []

Also, for the patch, would it have been clearer to submit the MR instead of the patch file? I will do so in the future.

xiaohua guan’s picture

xiaohua guan commented

@yas @RyoYamashita

I can't find the definition of K8sRestServiceInterface too. K8sRestServiceInterface should be included into the patch file. Also I think the name of the service class should be changed to K8sRestService.

ryo yamashita’s picture

ryo yamashita commented

@yas @xiaohua-guan

I submitted the fix by MR, not by patch file.
By creating the Factory class, I was able to reference the created K8sRestService class from the ApiController class.
However, when I acquire the log information, it becomes null.
The K8sRestService class should have acquired the K8sService information by DI in the constructor, but it may not have been read properly.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented

@RyoYamashita

Thank you for the update. I posted my comments.

@xiaohua-guan

What do you think?

xiaohua guan’s picture

xiaohua guan commented

@yas @RyoYamashita

I think the k8s rest service is unnecessary, because the APIController can call the getPodLog of k8s service directly. So the k8s rest service should be deleted.

ryo yamashita’s picture

ryo yamashita commented
Status: Needs work » Needs review

@yas

I have recreated a patch. Could you please check it?

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@ryo-yamashita

Thank you for the update and this code is what we expected. There is one leftover to add a comment into YAML file. Please see my comment at: https://git.drupalcode.org/project/cloud/-/merge_requests/338#note_39605

Thanks

ryo yamashita’s picture

ryo yamashita commented
Status: Needs work » Needs review

@yas

I fixed YAML file's comment and rebase this branch. Could you please check it?

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

@ryo-yamashita

Thank you for the update. It sounds great new. I'll merge the patch to 4.x and close this issue as Fixed.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Title: Add a REST API to get K8s Pod log » Add a REST API to get K8s Pod logs

  • yas committed 0b19d6f on 4.x authored by Ryo Yamashita 
    Issue #3227081 by Ryo Yamashita, yas, Xiaohua Guan: Add a REST API to...

yas closed merge request !338

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.