add option for peerPodOwnership #110
Conversation
Signed-off-by: Amrit Prakash <[email protected]>
There was a problem hiding this comment.
Hi @solo-daemon, thanks for the PR, it would be great to get a fix for this issue. I think the biggest piece of work will be to add some suitable tests, unfortunately there are any to build on yet- see #48. If you're happy to carry on working on this, I can assign the issue to you. Thanks again.
| @@ -448,7 +449,22 @@ func CreateChaincodePod( | |||
| chaincodeData, | |||
| ) | |||
|
|
|||
| err := deleteChaincodePod(ctx, logger, podsClient, podName, namespace, chaincodeData) | |||
| peerPod, err := podsClient.Get(ctx, peerID, metav1.GetOptions{}) | |||
There was a problem hiding this comment.
This doesn't look like the right way to get the peer pod. Instead, I think there needs to be something to look up the pod that the builder is running on, although I'm not sure how to do that off the top of my head.
There was a problem hiding this comment.
The idea here was that whatever namespace is being passed to the Create Chain code pod will be the only namespace from which (if a peer pod exist ) can be added as an owner and the podsClient passed is created from the same namespace.
There was a problem hiding this comment.
That makes sense, however I don't think there's currently any way to identify the peer pod. The builder would be running in the same container as the peer, so it would be in the same pod, but I've not found a way to get the current pod using the kubernetes client API (I'm probably just missing something obvious). The Downward API would work but that would add some requirements on the way the peer pod is defined, plus probably more config for the k8s builder.
|
I'm currently working on a PR to use a job to run chaincode which would resolve #88 (the pod's owner reference is now the job) and make this PR redundant. Thanks again for contributing to the k8s builder @solo-daemon. |
|
@jt-nti it was a pleasure, i was contributing because i wanted to learn go-lang reading the code sure helped a lot. Thanks! |
fix #88
I tried to fix the issue though I am new to golang and kubernetes both. Would love to improve this pr and get it merged!