feat(buildkit): support buildkit auto-discovery on Kubernetes #33
Conversation
nettoclaudio
commented
Mar 7, 2023
nettoclaudio
commented
- Auto-discovery BuildKit's pods on Kubernetes
- Lock the selected BuildKit pod using Leases
- Set/unset Tsuru app labels on the selected BuildKit pod (allowing the BuildKit pod to inherit the same NetworkPolicies as Tsuru app)
| @@ -64,24 +100,33 @@ func (b *BuildKit) Build(ctx context.Context, r *pb.BuildRequest, w io.Writer) ( | |||
| return nil, errors.New("writer must implement console.File") | |||
| } | |||
|
|
|||
| c, clientCleanUp, err := b.client(ctx, r) | |||
| if err != nil { | |||
There was a problem hiding this comment.
it is possible for b.client() to return an err and still leave something to cleanUp. We would need to call clientCleanUp() inside this if.
(eg: not cleaned things from function func (d *k8sDiscoverer) discoverBuildKitClientFromApp())
There was a problem hiding this comment.
Hum... For sure it's possible but I'm not comfortable with this approach. I usually discard any other return object when the error is not zero (empty). I'm not 100% but I think it's a Go convention BTW.
| OnStartedLeading: func(c context.Context) { | ||
| select { | ||
| case ch <- pod: | ||
| klog.V(4).Infof("Selected BuildKit pod: %s/%s", pod.Namespace, pod.Name) |
There was a problem hiding this comment.
adding the uniqueHolderName on this log could be useful (also, the 2 logs after this one)
There was a problem hiding this comment.
Honestly, this info (uniqueHolderName) doesn't help anything. This name it's mostly like a random number around each build call, it's important just to avoid conflict while getting the lease (lock). Two consecutive build calls will generate different holder names, so they will not share the lease lock.
