Closed Schnitzel closed 3 years ago
regarding kubernetes bug, looks like that it's actually a linux thing: https://github.com/kubernetes/kubernetes/issues/89906
its not possible to store a NUL-byte inside a linux environment variable
so I think that's ok therefore and we need to solve the issue on our side
I was able to get it running with: https://github.com/amazeeio/lagoon-kbd/pull/22 - pushed as amazeeio/lagoon-builddeploy:v0.1.6rc1
but honestly I'm not sure if that's the right way, the LagoonBuilds
object now looks like this:
pullrequest:
baseBranch: master
baseSha: 229ec3e57e220114e1c6ab60dde07a8ea12b873b
headBranch: pr-testing
headSha: 14627d2b6061ed1a1a1b8a692963145271a9a5d8
number: "78"
title: test2
which then creates a pod with
- name: PR_TITLE
value: test2
- name: PR_NUMBER
value: '78'
I tried to create a pod with
- name: PR_NUMBER
value: 78
and this actually doesn't work as kubectl errors:
Error from server (BadRequest): error when creating "/private/var/folders/xx/8zf5sz1d24n0cxtgsk956cvr0000gn/T/5c917a3e651ad4ef254f46d6784d17f2/resource.yaml": Pod in version "v1" cannot be handled as a Pod: v1.Pod.Spec: v1.PodSpec.Containers: []v1.Container: v1.Container.Env: []v1.EnvVar: v1.EnvVar.Value: ReadString: expects " or n, but found 7, error found in #10 byte of ...|,"value":78},{"name"|..., bigger context ...|LE","value":"test2"},{"name":"PR_NUMBER","value":78},{"name":"LAGOON_PROJECT_VARIABLES","value":"[{\|...
So I think this solution actually should be a solution
I'm more confused why the testing infra works though?
figured out why the testing does not fail, we actually never test pullrequests currently in amazeeio/lagoon or in amazeeio/lagoon-kbd:
https://github.com/amazeeio/lagoon/blob/2823f93897b034b7292c5d08f59ca761fb06897d/Makefile#L929
therefore this never was caught.
So I think we should merge this in.
Nice! I think we can go with this for now. I am concerned that Lagoon is sending the int as a string though.
When creating the EnvVar, the controller converts the int to string so that it gets created properly. For reference, the types for corev1.EnvVar
are Name:string, Value:string.
podEnvs = append(podEnvs, corev1.EnvVar{
Name: "PR_NUMBER",
Value: string(lagoonBuild.Spec.Pullrequest.Number),
})
The step that performs an unmarshalling of the JSON payload into a LagoonBuild won't know how to turn the string to an int, so gets confused. I think this is the reason that it was breaking is because the PR number in the spec wasn't being created properly as an int cause Lagoon was sending it as a string.
If Lagoon sent the PR number as an actual JSON int, it would have been fine.
I think Lagoon should try and do types properly where possible, if a value is an int, make sure the JSON representation being passed around is an int too.
I'm currently testing lagoon-master with lagoon-kbd against a kubernetes cluster and found the following issue:
Lagoon-master creates a rabbitmq message like this:
see the
spec.pullrequest.number
in therebut then the
LagoonBuild
object is created by lagoon-build-deploy and it disapears:see that now
spec.pullrequest.number
is missingthe pod that is created to deploy PRs then looks like this:
with
which then causes kubernetes to fail:
while I think this is actually a kubernetes bug that you should be able to create a pod with
\0
as env variable, it's also a bug that the pull request number gets somehow swallowed by the controller.I found the code that takes the message and creaetes the
LagoonBuilds
object: https://github.com/amazeeio/lagoon-kbd/blob/main/handlers/message_queue.go#L95-L106 and I don't see anything wrong with it.The only piece that I could imagine is that somehow the fact the value in the rabbitmq message is a string, but the lagoon types for
LagoonBuilds
is defined as a number: https://github.com/amazeeio/lagoon-kbd/blob/86935f85c0516ee50deff1839000f7567f6b1b53/api/v1alpha1/lagoonbuild_types.go#L142 could cause some issues?