Notes: brainstormed on proposing the plural version for klog.KObj, writing new validation checks in the upstream kube-linter #22

June 9, 2021

Quick notes from today:

brainstormed on proposing the plural version for klog.KObj

Couple of weeks back, I had picked up this issue Propose plural version for klog.KObj #102434 as part of a requirement in the structured logging migration work. Although, I didn’t assign the issue then, to myself because I wasn’t quite sure about the blueprint (structure of input arguments, & the output format) for this intended new function
So, as my starting point (during these last weeks), to understand the requirements of the feature, I went through understanding the previous implementation of the same function for singular arguments here ~ Implement klog.KObj klog.KRef functions #128

I was able to understand this previous singular functions klog.KObj & klog.KRef functions definition, but again that was it. I was still struggling to understand what would be the input structure for a similar plural function & so the intended output (as I already mentioned above).
So, I reached out to Marek Siarkowicz today (Tech Lead of the structured logging working group) to discuss it.

(Note: It took me eons to finally ask! Asking is always one of those things that make me uncomfortable, but taking this step always opens up thousand opportunities for me, right away!)
He started by asking me “did you have any ideas about the input structure?”

(And I was like, what should I answer now. This is exactly what I wanted to understand during this discussion. Although I realised very soon later, I should have put more work before asking as well.)
So, to answer the question, I spent some time digging in & reading the relevant discussion from this old PR ~ Refactor pods format to return ObjRef slice #99799.

From this PR, I got to learn about a previous attempt to support plural version of kog.KOj function. I found this test case (one of the file changes introduced by the PR) that I felt like, could serve as an input blueprint for the required plural version of the function.

The test case patch looked like this ~
```
testCases := []struct {
      caseName      string
      pods          []*v1.Pod
      expectedValue []klog.ObjectRef
  }{
      {"input_nil_case", nil, []klog.ObjectRef{}},
      {"input_empty_case", []*v1.Pod{}, []klog.ObjectRef{}},
      {"input_length_one_case", []*v1.Pod{pod1, nil}, []klog.ObjectRef{pod1Obj}},
      {"input_length_more_than_one_case", []*v1.Pod{pod1, pod2}, []klog.ObjectRef{pod1Obj, pod2Obj}},
      {"input_include_nil_case", []*v1.Pod{pod1, nil}, []klog.ObjectRef{pod1Obj}},
  }
```
So, as I see from this test patch,
- The testCases go variable is a slice (go array) of structure having struct variables as the caseName, pods (which is k8s pod object list, so the plural input, i.e. in place of single pod object as in case of klog.kObj function, here we have a list of pod objects), & finally the expectedValue (again the plural output i.e a list/slice of object references).
- Taking one of the value of this struct slice as an example ~
  
  {"input_length_more_than_one_case", []*v1.Pod{pod1, pod2}, []klog.ObjectRef{pod1Obj, pod2Obj}}, I understood:
  - My input (if I take the above example while defining my plural function) would look something like this slice of kObj (in this case, a slice of pod k8s objects) ~ []*v1.Pod{pod1, pod2}
  - And so, the output would be a slice of kObjRef, looking something like ~ []klog.ObjectRef{pod1Obj, pod2Obj}}
But that’s not it.
- As stated in the description of this issue Propose plural version for klog.KObj #102434,
  
  In golang, slice of struct cannot be passed into slice of interfaces they implement. This is due to difference in memory representation of two slices (cant wait for Generics).
  
  For example ~ []corev1.Pods{} cannot be passed to func KObjs(objs []klog.ObjectRef)
So, beause of the above stated problem, the above test case implementation was later changed to something like this ~
```
testCases := []struct {
      caseName      string
      pods          []*v1.Pod
      expectedValue string
  }{
      {"input_nil_case", nil, ""},
      {"input_empty_case", []*v1.Pod{}, ""},
      {"input_length_one_case", []*v1.Pod{pod1}, "pod1_default(551f5a43-9f2f-11e7-a589-fa163e148d75):DeletionTimestamp=2017-09-26T14:37:50Z"},
      {"input_length_more_than_one_case", []*v1.Pod{pod1, pod2}, "pod1_default(551f5a43-9f2f-11e7-a589-fa163e148d75):DeletionTimestamp=2017-09-26T14:37:50Z, pod2_default(e84a99bf-d1f9-43c2-9fa5-044ac85f794b):DeletionTimestamp=2017-09-26T14:37:50Z"},
      {"input_include_nil_case", []*v1.Pod{pod1, nil}, "pod1_default(551f5a43-9f2f-11e7-a589-fa163e148d75):DeletionTimestamp=2017-09-26T14:37:50Z, <nil>"},
  }
```
that is ~
- in place of []klog.ObjectRef{pod1Obj, pod2Obj}}, the output now looks something like ~ "pod1_default(551f5a43-9f2f-11e7-a589-fa163e148d75):DeletionTimestamp=2017-09-26T14:37:50Z, pod2_default(e84a99bf-d1f9-43c2-9fa5-044ac85f794b):DeletionTimestamp=2017-09-26T14:37:50Z"}
So, yea, all of this above is what I answered to Marek to the question he asked about the input structure.

(Note that, this was the very first time I was able to decipher even this much. So, that is why when I said above I should have worked a little more, I meant about all this above ^)
Marek very kindly added the following to make things even more clear.

To provide a consistent way of representing Kubernetes Objects in logs (Pods, Secrets, Nodes etc) we have created dedicated functions like klog.KObj. This works for single objects as contributor can just call klog.InfoS("message", "pod", klog.KObj(pod), "secret", klog.KObj(secret)) and they are done. One function that can handle any object type.

This is thanks to fact that all Kubernetes objects implement GetName() and GetNamespace() methods, so we can simply accept an interface.

The problem is with slices of objects, we don’t have any method klog.KObjs, ultimately requiring contributors to implement it themselves (like format.Pods).

So, the gal is to introduce this function klog.KObjs.

I was very happy with this explanation. Quite a lot of things made so much sense now.
Marek also added that

logging library and all the methods needed to write logs should be in upstream klog source code.

It creates one place where contributors can search and find all functions needed to write logs.

Fact that we have some helper methods spread around kubernetes/kubernetes repository is bad, as it leads to worse discover-ability and diverging log format.

We want consistant log format, as it will be much easier for consumer of logs just to handle one representation of Pods, instead of having big switch statements to differently read Pod info in kube-apiserver than kube-controller-manager.

So, the final outcome of this discussion was the following ~

The blueprint of the new function would look like:

testCases := []struct {
     caseName                 string
     generalised_objects      <list of any k8s objects>
     expectedValue            []klog.ObjectRef
 }{
     {"input_nil_case", nil, []klog.ObjectRef{}},
     {"input_empty_case", <list of any k8s objects>, []klog.ObjectRef{}},
     {"input_length_one_case", <list of any k8s objects>, []klog.ObjectRef{object1},
     {"input_length_more_than_one_case", <list of any k8s objects>, []klog.ObjectRef{object1, object2}},
     {"input_include_nil_case", <list of any k8s objects>, []klog.ObjectRef{object1}},
 }

that is, in place of just the pod object lists, as in the above cases, we need something as one kObjs fucntion that’s generic to all the k8s objects.

and all this will implemented in the upstream klog library source code.

writing new validation checks in the upstream kube-linter

(I also had this another small win today. I was kind of under a work block so, was finding so hard to even start, but again I learnt it’s the first 10 min that is the hard part & provides the highest resistence. Following that, it will happen! I need to work really hard at bringing this in practice!)

We’ve decided to write the validation checks (that were previously meant to be written under DVO as I mentioned in one of the previous blogs here) in the upstream kube-linter project itself. That is to remove the redundant part of maintaining the checks in a consumer project & ofcourse, to help grow the upstream library validation checks.
I’ve picked up the following two issues & I’m currently working on them ~
- [FEATURE_REQUEST] Add check for trusted sources #182
- [FEATURE_REQUEST] Add check for trusted tags #183
I was able to work today (& just a little yesterday too. That is when I realised, we’re now doing the new checks in the upstream project only)

So, I had set up kube-linter project locally, & currently trying to implement the check for trusted container image tags.

That’s all for today. Happy that I finally managed to get back to writing. 🙂

I speak here {^_^}