renoki-co / laravel-php-k8s

Just a simple port of renoki-co/php-k8s for easier access in Laravel
Apache License 2.0
86 stars 7 forks source link

Macros are not working using fromYaml() with custom CRDs #12

Closed palcamora closed 3 years ago

palcamora commented 3 years ago

I have defined the Macros inside KubernetesProvider.

K8s::macro('gateway', function ($cluster = null, array $attributes = []){ return new IstioGateway($cluster, $attributes); });

And when I call K8s::connection(1)->gateway->whatever->create(); it works properly. But if I try to recover a [kind: gateway] from yaml (using it like this:

K8s::connection(1)
                        ->fromYaml($content)
                        ->create();

It throws the following error:

[2021-09-13 10:54:54] local.ERROR: Undefined property: RenokiCo\PhpK8s\K8s::$cluster {"exception":"[object] (ErrorException(code: 0): Undefined property: RenokiCo\PhpK8s\K8s::$cluster at /home/vagrant/backend/vendor/renoki-co/php-k8s/src/K8s.php:427) [stacktrace]

0 /home/vagrant/backend/vendor/renoki-co/php-k8s/src/K8s.php(427): Laravel\Lumen\Application->Laravel\Lumen\Concerns\{closure}()

1 /home/vagrant/backend/app/Observers/ConfigurationObserver.php(24): RenokiCo\PhpK8s\K8s->__call()

2 /home/vagrant/backend/vendor/illuminate/events/Dispatcher.php(404): App\Models\Configuration::App\Observers\{closure}()

3 /home/vagrant/backend/vendor/illuminate/events/Dispatcher.php(249): Illuminate\Events\Dispatcher->Illuminate\Events\{closure}()

4 /home/vagrant/backend/vendor/illuminate/database/Eloquent/Concerns/HasEvents.php(189): Illuminate\Events\Dispatcher->dispatch()

5 /home/vagrant/backend/vendor/illuminate/database/Eloquent/Model.php(1166): Illuminate\Database\Eloquent\Model->fireModelEvent()

6 /home/vagrant/backend/vendor/illuminate/database/Eloquent/Model.php(986): Illuminate\Database\Eloquent\Model->performInsert()

7 /home/vagrant/backend/app/Http/Controllers/Kubernetes/DeploymentController.php(236): Illuminate\Database\Eloquent\Model->save()

8 /home/vagrant/backend/app/Http/Controllers/Kubernetes/DeploymentController.php(55): App\Http\Controllers\Kubernetes\DeploymentController->createConfigurations()

9 /home/vagrant/backend/vendor/illuminate/container/BoundMethod.php(36): App\Http\Controllers\Kubernetes\DeploymentController->store()

10 /home/vagrant/backend/vendor/illuminate/container/Util.php(40): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()

11 /home/vagrant/backend/vendor/illuminate/container/BoundMethod.php(93): Illuminate\Container\Util::unwrapIfClosure()

12 /home/vagrant/backend/vendor/illuminate/container/BoundMethod.php(37): Illuminate\Container\BoundMethod::callBoundMethod()

13 /home/vagrant/backend/vendor/illuminate/container/Container.php(651): Illuminate\Container\BoundMethod::call()

14 /home/vagrant/backend/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(389): Illuminate\Container\Container->call()

15 /home/vagrant/backend/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(355): Laravel\Lumen\Application->callControllerCallable()

16 /home/vagrant/backend/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(329): Laravel\Lumen\Application->callLumenController()

17 /home/vagrant/backend/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(282): Laravel\Lumen\Application->callControllerAction()

18 /home/vagrant/backend/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(267): Laravel\Lumen\Application->callActionOnArrayBasedRoute()

19 /home/vagrant/backend/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(169): Laravel\Lumen\Application->handleFoundRoute()

20 /home/vagrant/backend/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(429): Laravel\Lumen\Application->Laravel\Lumen\Concerns\{closure}()

21 /home/vagrant/backend/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(175): Laravel\Lumen\Application->sendThroughPipeline()

22 /home/vagrant/backend/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(112): Laravel\Lumen\Application->dispatch()

23 /home/vagrant/backend/public/index.php(28): Laravel\Lumen\Application->run()

24 {main}

"}

Looking inside K8s file, I realised thath the problem it with macros.

/**
     * Proxy the K8s call to cluster object.
     *
     * @param  string  $method
     * @param  array  $parameters
     * @return mixed
     */
    public function __call($method, $parameters)
    {
        if (static::hasMacro($method)) {
            return $this->macroCall($method, $parameters);
        }

        return $this->cluster->{$method}(...$parameters);
    }

It is calling $this->cluster, instead of $this->macroCall, because it does not recognize de macros calling in this way. It is pretty strange and I dont know how to resolve this issue.

rennokki commented 3 years ago

Please check the documentation. It is a small confusion. ->fromYaml() needs to ALWAYS be called from a KubernetesCluster instance

Please let me know if this way works. I have a small doubt that it might not work with macros since the initialization from YAML to resource might not be seen properly.

palcamora commented 3 years ago

Thanks for your answer.

I call fromYaml() from a KubernetesCluster instance, that is called from a LaravelFacade that is initializated in a provider, and it dont work. As I said before, using K8s::gateway() works correcly, and genrates a GatewayKind Instance, but using fromYaml, generates this error when ->create is executed, and generates an K8s instance when not, instead of the GatewayKind that should be create.

I tried to use the original KubernetesCluster class (use RenokiCo\PhpK8s\KubernetesCluster;), and use it like this:

$cluster = new KubernetesCluster('http://127.0.0.1:8080');
$cluster->fromKubeConfigYaml(Cluster::find(1)->kubeconfig, 'pre');

K8s::macro('gateway', function ($cluster = null, array $attributes = []){
    return new IstioGateway($cluster, $attributes);
});

return dd(
$cluster->fromYaml(array2yaml(yaml_parse(Configuration::find(38)->content)))
);

**The K8s class here is RenokiCo\PhpK8s\K8s

And this is still generating a RenokiCo\PhpK8s\K8s instance instead of a IstioGateway instance (same error as using the facade)

If you prefer, I can create an empty laravel project and show you the completly code I use in order to reproduce the error.

rennokki commented 3 years ago

For some reason, I saw k8s in the repo name and I assumed it's php-k8s. 😅

Before calling it a bug, make sure to call the macro on RenokiCo\PhpK8s\K8s instance and make sure that the macro name matches the class name for the gateway, in this case, istioGateway:

use RenokiCo\PhpK8s\K8s as PhpK8s;

PhpK8s::macro('istioGateway', function ($cluster = null, array $attributes = []) {
    return new IstioGateway($cluster, $attributes);
});
rennokki commented 3 years ago

This can be some good feature to better help to define CRDs 🤔 Just by passing the class name and having a macro set up instead of having to do it your own. 🤩

use RenokiCo\PhpK8s\K8s as PhpK8s;

PhpK8s::crd(IstioGateway::class);
palcamora commented 3 years ago

I still have the same issue. I hace created an empty lumen installation and I will show you all the relevant files that I have write to reproduce this issue.

bootstrap/app.php

$app->register(App\Providers\AppServiceProvider::class);
// $app->register(App\Providers\AuthServiceProvider::class);
// $app->register(App\Providers\EventServiceProvider::class);
$app->register(\RenokiCo\LaravelK8s\LaravelK8sServiceProvider::class);

App\Providers\AppServiceProvider

<?php

namespace App\Providers;

use Illuminate\Support\ServiceProvider;
use RenokiCo\PhpK8s\K8s;

use App\Kubernetes\Kinds\IstioGateway;

class AppServiceProvider extends ServiceProvider
{
    /**
     * Bootstrap services.
     *
     * @return void
     */
    public function boot()
    {
        K8s::macro('istioGateway', function ($cluster = null, array $attributes = []){
            return new IstioGateway($cluster, $attributes);
        });
    }
}

IstioGateway

<?php

namespace App\Kubernetes\Kinds;

use RenokiCo\PhpK8s\Contracts\InteractsWithK8sCluster;
use RenokiCo\PhpK8s\Kinds\K8sResource;

class IstioGateway extends K8sResource implements InteractsWithK8sCluster
{
    /**
     * The resource Kind parameter.
     *
     * @var null|string
     */
    protected static $kind = 'Gateway';

    /**
     * The default version for the resource.
     *
     * @var string
     */
    protected static $defaultVersion = 'networking.istio.io/v1beta1';

    /**
     * Wether the resource has a namespace.
     *
     * @var bool
     */
    protected static $namespaceable = true;
}

ExampleController

<?php

namespace App\Http\Controllers;

use RenokiCo\LaravelK8s\LaravelK8sFacade as K8sFacade;

class ExampleController extends Controller
{
    /**
     * Create a new controller instance.
     *
     * @return void
     */
    public function __construct()
    {
        //
    }

    public function setNamespace() {
        return K8sFacade::namespace()
            ->setName('renoki-test')
            ->setLabels([
                'istio-injection' => 'enabled'
            ])->create();
    }

    public function getNamespace() {
        $yaml = $this->array2yaml(K8sFacade::namespace()
        ->setName('renoki-test2')
        ->setLabels([
            'istio-injection' => 'enabled'
        ])->toArray());

        dd(K8sFacade::fromYaml($yaml));

    }

    public function setGateway() {
        return K8sFacade::istioGateway()
                    ->setName('test-gateway')
                    ->setNamespace('renoki-test')
                    ->setSpec([
                        'selector' => [
                            'istio' => 'ingressgateway',
                        ],
                        'servers' => [
                            [
                                'hosts' => 'test.gateway.io',
                                'port' => [
                                    'name' => 'https',
                                    'number' => 443,
                                    'protocol' => 'HTTPS'
                                ],
                                'tls' => [
                                    'credentialName' => 'kcertificate',
                                    'mode'  => 'SIMPLE'
                                ]
                            ]
                        ]
                    ])->create();
    }

    public function getGateway() {
        $yaml = $this->array2yaml(K8sFacade::istioGateway()
                    ->setName('test-gateway2')
                    ->setNamespace('renoki-test2')
                    ->setSpec([
                        'selector' => [
                            'istio' => 'ingressgateway',
                        ],
                        'servers' => [
                            [
                                'hosts' => 'test.gateway.io',
                                'port' => [
                                    'name' => 'https',
                                    'number' => 443,
                                    'protocol' => 'HTTPS'
                                ],
                                'tls' => [
                                    'credentialName' => 'kdashboard-certificate',
                                    'mode'  => 'SIMPLE'
                                ]
                            ]
                        ]
                    ])->toArray());
            dd(K8sFacade::fromYaml($yaml));
    }

    private function array2yaml(array $arr)
    {
        $yaml = yaml_emit($arr);
        return str_replace("---\n", "", $yaml);
    }
}

Now I will explain the behaviour:

setNamespace() and setGateway() works perfectly. No errors are thrown and both of theme are created on cluster.

getNamespace() also works properly. It reads the yaml and generate a K8sNamespace instance, so if I try to create this other namespace, it will create this correctly too.

dd() result

^ RenokiCo\PhpK8s\Kinds\K8sNamespace {#46 ▼
  #attributes: array:1 [▼
    "metadata" => array:2 [▼
      "name" => "renoki-test"
      "labels" => array:1 [▶]
    ]
  ]
  #original: array:1 [▼
    "metadata" => array:2 [▼
      "name" => "renoki-test"
      "labels" => array:1 [▶]
    ]
  ]
  #synced: false
  #cluster: RenokiCo\PhpK8s\KubernetesCluster {#18 ▶}
}

getGateway() makes the problem. In this way, it generates a K8s instance, instead of an IstioGateway instance.

dd() result

^ RenokiCo\PhpK8s\K8s {#46}

And when I add create() It throws this error

public function getGateway() {
        $yaml = $this->array2yaml(K8sFacade::istioGateway()
                    ->setName('test-gateway2')
                    ->setNamespace('renoki-test2')
                    ->setSpec([
                        'selector' => [
                            'istio' => 'ingressgateway',
                        ],
                        'servers' => [
                            [
                                'hosts' => 'test.gateway.io',
                                'port' => [
                                    'name' => 'https',
                                    'number' => 443,
                                    'protocol' => 'HTTPS'
                                ],
                                'tls' => [
                                    'credentialName' => 'kdashboard-certificate',
                                    'mode'  => 'SIMPLE'
                                ]
                            ]
                        ]
                    ])->toArray());

          // dd(K8sFacade::fromYaml($yaml));
          return K8sFacade::fromYaml($yaml)->create();

[2021-09-15 09:08:11] local.ERROR: Undefined property: RenokiCo\PhpK8s\K8s::$cluster {"exception":"[object] (ErrorException(code: 0): Undefined property: RenokiCo\PhpK8s\K8s::$cluster at /home/vagrant/lumen/vendor/renoki-co/php-k8s/src/K8s.php:115) [stacktrace]

0 /home/vagrant/lumen/vendor/renoki-co/php-k8s/src/K8s.php(115): Laravel\Lumen\Application->Laravel\Lumen\Concerns\{closure}()

1 /home/vagrant/lumen/app/Http/Controllers/ExampleController.php(88): RenokiCo\PhpK8s\K8s->__call()

2 /home/vagrant/lumen/vendor/illuminate/container/BoundMethod.php(36): App\Http\Controllers\ExampleController->getGateway()

3 /home/vagrant/lumen/vendor/illuminate/container/Util.php(40): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()

4 /home/vagrant/lumen/vendor/illuminate/container/BoundMethod.php(93): Illuminate\Container\Util::unwrapIfClosure()

5 /home/vagrant/lumen/vendor/illuminate/container/BoundMethod.php(37): Illuminate\Container\BoundMethod::callBoundMethod()

6 /home/vagrant/lumen/vendor/illuminate/container/Container.php(651): Illuminate\Container\BoundMethod::call()

7 /home/vagrant/lumen/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(389): Illuminate\Container\Container->call()

8 /home/vagrant/lumen/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(355): Laravel\Lumen\Application->callControllerCallable()

9 /home/vagrant/lumen/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(329): Laravel\Lumen\Application->callLumenController()

10 /home/vagrant/lumen/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(282): Laravel\Lumen\Application->callControllerAction()

11 /home/vagrant/lumen/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(267): Laravel\Lumen\Application->callActionOnArrayBasedRoute()

12 /home/vagrant/lumen/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(169): Laravel\Lumen\Application->handleFoundRoute()

13 /home/vagrant/lumen/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(429): Laravel\Lumen\Application->Laravel\Lumen\Concerns\{closure}()

14 /home/vagrant/lumen/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(175): Laravel\Lumen\Application->sendThroughPipeline()

15 /home/vagrant/lumen/vendor/laravel/lumen-framework/src/Concerns/RoutesRequests.php(112): Laravel\Lumen\Application->dispatch()

16 /home/vagrant/lumen/public/index.php(28): Laravel\Lumen\Application->run()

17 {main}

"}

Of course I have tried all the combinations that you told me before, but it still have this weird behaviour, and even checking the original code, I am not able to explain what is happening 😓.

I expect this explanation become more helpfull.

Thanks you

rennokki commented 3 years ago

It seems like the YAML gets declared with kind: Gateway and it searches for a gateway method:

- PhpK8s::macro('istioGateway', function ($cluster = null, array $attributes = []) {
+ PhpK8s::macro('gateway', function ($cluster = null, array $attributes = []) {
    return new IstioGateway($cluster, $attributes);
});

I think this behavior needs to be changed accordingly so that you can declare the naming as a convention for both YAML-imported and creating instances also based on namespaces.

In this case, Gateway is pretty vague, and having a K8s::gateway() might interfere with another CRDs, so I think the K8s calls for resources should be somehow namespaced. I thought of having the following example, but it sucks since you need to define the function in the YAML file:

kind: Gateway
apiVersion: networking.istio.io/v1beta1
phpK8s:
  macro: istioGateway
PhpK8s::macro('istioGateway', function ($cluster = null, array $attributes = []) {
    return new IstioGateway($cluster, $attributes);
});
rennokki commented 3 years ago

Maybe having annotations would look better (and would fix the issues with the YAML validation) ?

kind: Gateway
apiVersion: networking.istio.io/v1beta1
metadata:
  name: my-gateway
  annotations:
    php-k8s.renoki.org/macro: istioGateway
palcamora commented 3 years ago

If you remember, I normally use this

 PhpK8s::macro('gateway', function ($cluster = null, array $attributes = []) {
    return new IstioGateway($cluster, $attributes);
});

But with the same result.

And yes, the way with annotations looks great for me 😁

rennokki commented 3 years ago

I might pluck the namespace for the apiVersion and instead use it to define a macro internally so that any version from networking.istio.io/*/gateway to point out where it should be.

It's weird that I have just tested locally in this configuration and it worked. Let me put up a test for it and open a PR.

palcamora commented 3 years ago

Okay, thanks you

palcamora commented 3 years ago

Okey, I just realized what is the problem 😅

As kind: Gateway is with uppercase, I have to declare the macro also with uppercas, like this.

PhpK8s::macro('Gateway', function ($cluster = null, array $attributes = []) {
    return new IstioGateway($cluster, $attributes);
});
rennokki commented 3 years ago

Yes, that's an issue actually 🤔 I think we can make the kind to be called as camelCase.

rennokki commented 3 years ago

It should be fixed as of 3.1.1 when I'll release it. So the approach is the following:

The class name or namespace doesn't matter anymore. You can call IstioGateway::register() to register the macro calls for the CRD. Automatically, will be created a macro for you, and an internal macro named by the first bit (the vendor one) of $defaultVersion and the $kind. So in this case, it's networkingistioiogateway. This macro will be used by the fromYaml() function to be able to identify which macro to call. It will take the passed kind and apiVersion from it and will rebuild the same macro. The macro will be then called.

The test is here.

When calling IstioGateway::register(), you may use the $cluster->istioGateway()->... instead of the current workaround (Gateway capitalized).

Delete the macro declaration and replace it with the register method. You can see in the test the pretty simple approach.

rennokki commented 3 years ago

I fixed it in PHP K8s 3.1.1: https://github.com/renoki-co/php-k8s/releases/tag/3.1.1 Laravel PHP K8s 3.1.0 meets the requirements to update to PHP K8s 3.1+ (https://github.com/renoki-co/laravel-php-k8s/releases/tag/3.1.0), you might want to bump that version up.

I have also updated the docs: https://php-k8s.renoki.org/advanced/create-classes-for-crds/macros

palcamora commented 3 years ago

Okay I am updating, thanks you!

Another little thing.

I use the array2yaml() method cause yaml_emit() do the same thing but it give the yaml in this format:

---
apiVersion: networking.istio.io/v1beta1
kind: Gateway
metadata:
  name: test-gateway2
  namespace: renoki-test2
spec:
  selector:
    istio: ingressgateway
  servers:
  - hosts: test.gateway.io
    port:
      name: https
      number: 443
      protocol: HTTPS
    tls:
      credentialName: kcertificate
      mode: SIMPLE
...

And when I execute fromYaml(), it give me an error cause is reading the --- of the begining of yaml and asuming that there are 2 different yaml kinds to deal. Maybe even a toYaml() method should be great too.

Thanks you again

rennokki commented 3 years ago

This is the behavior: https://www.php.net/manual/en/function.yaml-emit.php PHP K8s can import YAML without issues even if the break characters (---) are present.

rennokki commented 3 years ago

Just replaced the function with yaml_emit and they work (locally for now) like charm: https://github.com/renoki-co/php-k8s/blob/master/tests/YamlTest.php#L87-L121