Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
palcamora opened this issue Sep 13, 2021 · 17 comments
Closed

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

palcamora opened this issue Sep 13, 2021 · 17 comments
Assignees
Labels
question Further information is requested

Comments

@palcamora
Copy link

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
Copy link
Member

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.

@rennokki rennokki self-assigned this Sep 13, 2021
@rennokki rennokki added the question Further information is requested label Sep 13, 2021
@palcamora
Copy link
Author

palcamora commented Sep 13, 2021

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:https://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
Copy link
Member

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Member

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
Copy link
Author

palcamora commented Sep 15, 2021

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
Copy link
Member

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
Copy link
Author

Okay, thanks you

@palcamora
Copy link
Author

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
Copy link
Member

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

@rennokki
Copy link
Member

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants