oomichi / try-kubernetes

12 stars 5 forks source link

Define the way how to add e2e test for improving the coverage #29

Closed oomichi closed 5 years ago

oomichi commented 6 years ago
  1. Stable APIからテストされていないものをピックアップ apisnoop: https://github.com/cncf/apisnoop#why に表示されている Spreadsheet をクリックする。 色(緑、青、赤)がついていない項目がテストされていないAPIとなっており、levelがstableになっているものを1つ選ぶ。
  2. kubectl コマンドで該当するサブコマンドの存在を確認する。 2.1 なければ、該当するサブコマンドを kubectl に追加する。 REST API把握として、swagger.jsonのdescription、curl コマンドによる API動作確認を行う
oomichi commented 6 years ago

e2e テストの構造を把握し、REST API部分を特定する。 ~/go/src/k8s.io/kubernetes/test/e2e/apps/statefulset.go

  50 // GCE Quota requirements: 3 pds, one per stateful pod manifest declared above.
  51 // GCE Api requirements: nodes and master need storage r/w permissions.
  52 var _ = SIGDescribe("StatefulSet", func() {
  53         f := framework.NewDefaultFramework("statefulset")
  54         var ns string
  55         var c clientset.Interface
  56
  57         BeforeEach(func() {
  58                 c = f.ClientSet
  59                 ns = f.Namespace.Name
  60         })
  61
  62         framework.KubeDescribe("Basic StatefulSet functionality [StatefulSetBasic]", func() {
..
  90
  91                 // This can't be Conformance yet because it depends on a default
  92                 // StorageClass and a dynamic provisioner.
  93                 It("should provide basic identity", func() {   ※テスト項目
  94                         By("Creating statefulset " + ssName + " in namespace " + ns)
  95                         *(ss.Spec.Replicas) = 3
  96                         sst := framework.NewStatefulSetTester(c)
  97                         sst.PauseNewPods(ss)
  98
  99                         _, err := c.AppsV1().StatefulSets(ns).Create(ss)
 100                         Expect(err).NotTo(HaveOccurred())
 101
 102                         By("Saturating stateful set " + ss.Name)
 103                         sst.Saturate(ss)
 104
 105                         By("Verifying statefulset mounted data directory is usable")
 106                         framework.ExpectNoError(sst.CheckMount(ss, "/data"))
 107
 108                         By("Verifying statefulset provides a stable hostname for each pod")
 109                         framework.ExpectNoError(sst.CheckHostname(ss))
 110
 111                         By("Verifying statefulset set proper service name")
 112                         framework.ExpectNoError(sst.CheckServiceName(ss, headlessSvcName))
 113
 114                         cmd := "echo $(hostname) > /data/hostname; sync;"
 115                         By("Running " + cmd + " in all stateful pods")
 116                         framework.ExpectNoError(sst.ExecInStatefulPods(ss, cmd))
 117
 118                         By("Restarting statefulset " + ss.Name)
 119                         sst.Restart(ss)
 120                         sst.WaitForRunningAndReady(*ss.Spec.Replicas, ss)
 121
 122                         By("Verifying statefulset mounted data directory is usable")
 123                         framework.ExpectNoError(sst.CheckMount(ss, "/data"))
 124
 125                         cmd = "if [ \"$(cat /data/hostname)\" = \"$(hostname)\" ]; then exit 0; else exit 1; fi"
 126                         By("Running " + cmd + " in all stateful pods")
 127                         framework.ExpectNoError(sst.ExecInStatefulPods(ss, cmd))
 128                 })
 .. 別のテスト項目

テスト項目内の Restart が API 処理をやっているように見えるので、その実態を調査 test/e2e/framework/statefulset_utils.go

234 // Restart scales ss to 0 and then back to its previous number of replicas.
235 func (s *StatefulSetTester) Restart(ss *apps.StatefulSet) {
236         oldReplicas := *(ss.Spec.Replicas)
237         ss, err := s.Scale(ss, 0)
238         ExpectNoError(err)
239         // Wait for controller to report the desired number of Pods.
240         // This way we know the controller has observed all Pod deletions
241         // before we scale it back up.
242         s.WaitForStatusReplicas(ss, 0)
243         s.update(ss.Namespace, ss.Name, func(ss *apps.StatefulSet) { *(ss.Spec.Replicas) = oldReplicas })
244 }

test/e2e/framework/statefulset_utils.go

200 // Scale scales ss to count replicas.
201 func (s *StatefulSetTester) Scale(ss *apps.StatefulSet, count int32) (*apps.StatefulSet, error) {
202         name := ss.Name
203         ns := ss.Namespace
204
205         Logf("Scaling statefulset %s to %d", name, count)
206         ss = s.update(ns, name, func(ss *apps.StatefulSet) { *(ss.Spec.Replicas) = count })  ここで count で更新
207
208         var statefulPodList *v1.PodList
209         pollErr := wait.PollImmediate(StatefulSetPoll, StatefulSetTimeout, func() (bool, error) {
210                 statefulPodList = s.GetPodList(ss)
211                 if int32(len(statefulPodList.Items)) == count {
212                         return true, nil
213                 }
214                 return false, nil
215         })
216         if pollErr != nil {
217                 unhealthy := []string{}
218                 for _, statefulPod := range statefulPodList.Items {
219                         delTs, phase, readiness := statefulPod.DeletionTimestamp, statefulPod.Status.Phase, podutil.IsPodReady(&statefulPod)
220                         if delTs != nil || phase != v1.PodRunning || !readiness {
221                                 unhealthy = append(unhealthy, fmt.Sprintf("%v: deletion %v, phase %v, readiness %v", statefulPod.Name, delTs, phase, read
    iness))
222                         }
223                 }
224                 return ss, fmt.Errorf("Failed to scale statefulset to %d in %v. Remaining pods:\n%v", count, StatefulSetTimeout, unhealthy)
225         }
226         return ss, nil
227 }

update メソッド

246 func (s *StatefulSetTester) update(ns, name string, update func(ss *apps.StatefulSet)) *apps.StatefulSet {
247         for i := 0; i < 3; i++ {
248                 ss, err := s.c.AppsV1().StatefulSets(ns).Get(name, metav1.GetOptions{})
249                 if err != nil {
250                         Failf("failed to get statefulset %q: %v", name, err)
251                 }
252                 update(ss)
253                 ss, err = s.c.AppsV1().StatefulSets(ns).Update(ss)
254                 if err == nil {
255                         return ss
256                 }
257                 if !apierrs.IsConflict(err) && !apierrs.IsServerTimeout(err) {
258                         Failf("failed to update statefulset %q: %v", name, err)
259                 }
260         }
261         Failf("too many retries draining statefulset %q", name)
262         return nil
263 }

s.c.AppsV1().StatefulSets(ns).Update(ss)の実態を調査

 19 import (
 ..
 41         clientset "k8s.io/client-go/kubernetes"
 ..
 73 // StatefulSetTester is a struct that contains utility methods for testing StatefulSet related functionality. It uses a
 74 // clientset.Interface to communicate with the API server.
 75 type StatefulSetTester struct {
 76         c clientset.Interface
 77 }

vendor/k8s.io/client-go/kubernetes/typed/apps/v1/statefulset.go

111 // Update takes the representation of a statefulSet and updates it. Returns the server's representation of the statefulSet, and an error, if there is any    .
112 func (c *statefulSets) Update(statefulSet *v1.StatefulSet) (result *v1.StatefulSet, err error) {
113         result = &v1.StatefulSet{}
114         err = c.client.Put().
115                 Namespace(c.ns).
116                 Resource("statefulsets").
117                 Name(statefulSet.Name).
118                 Body(statefulSet).
119                 Do().
120                 Into(result)
121         return
122 }

vendor/k8s.io/client-go/kubernetes/typed/apps/v1/statefulset.go ファイルは kubernetes/client-go で開発されており、k/k に import されたもの。 つまり、k/k の e2e テストコードは最終的には k/client-go の REST API クライアントコードによって REST API のオペレーションが行われる。

oomichi commented 6 years ago

ここでは追加対象として GET /apis/extensions を選ぶ。 kubectl コマンドに該当しそうなサブコマンドは存在しなかった。

$ kubectl get extensions
error: the server doesn't have a resource type "extensions"

そこで Swagger Specのdescriptionを確認する。

47858    "/apis/extensions/": {
47859     "get": {
47860      "description": "get information of a group",

この場合、まずは k/client-go に対象 REST API コードを追加してから、k/k に e2e テストコードを追加する。 curl で API挙動を確認する。 https://kubernetes.io/docs/tasks/access-application-cluster/access-cluster/#directly-accessing-the-rest-api

$ APISERVER=$(kubectl config view --minify | grep server | cut -f 2- -d ":" | tr -d " ")
$ TOKEN=$(kubectl describe secret $(kubectl get secrets | grep ^default | cut -f1 -d ' ') | grep -E '^token' | cut -f2 -d':' | tr -d " ")
$ curl $APISERVER/apis/extensions --header "Authorization: Bearer $TOKEN" --insecure
{
  "kind": "APIGroup",
  "apiVersion": "v1",
  "name": "extensions",
  "versions": [
    {
      "groupVersion": "extensions/v1beta1",
      "version": "v1beta1"
    }
  ],
  "preferredVersion": {
    "groupVersion": "extensions/v1beta1",
    "version": "v1beta1"
  }
}

extensionsとして有効になっている API バージョンを返している。 ここでは v1beta1 のみを返しており、これを URI にくっつけて Call すると有効な extensions 一覧が返ってくる。

$ curl $APISERVER/apis/extensions/v1beta1 --header "Authorization: Bearer $TOKEN" --insecure
{
  "kind": "APIResourceList",
  "groupVersion": "extensions/v1beta1",
  "resources": [
    {
      "name": "daemonsets",
      "singularName": "",
      "namespaced": true,
      "kind": "DaemonSet",
      "verbs": [
        "create",
        "delete",
        "deletecollection",
        "get",
        "list",
        "patch",
        "update",
        "watch"
      ],
      "shortNames": [
        "ds"
      ]
    },
    {
      "name": "daemonsets/status",
      "singularName": "",
      "namespaced": true,
      "kind": "DaemonSet",
      "verbs": [
        "get",
        "patch",
        "update"
      ]
    },
    {
      "name": "deployments",
      "singularName": "",
      "namespaced": true,
      "kind": "Deployment",
      "verbs": [
        "create",
        "delete",
        "deletecollection",
        "get",
        "list",
        "patch",
        "update",
        "watch"
      ],
      "shortNames": [
        "deploy"
      ]
    },
...
oomichi commented 6 years ago

client-go に GET /apis/extensions に該当するクライアントコードを追加しようとしたところ、 /client-go/kubernetes/typed/extensions/v1beta1/deployment.go に

 17 // Code generated by client-gen. DO NOT EDIT.

とあった。ほかにもメタデータを定義するリポジトリがあり、そのデータを使ってコード生成を 行っている可能性がある。実装方法を調べ、それに即した形で実装を行う。 → 自動生成したと思われるクライアントコードのパス配下にテストコードが無いのもそのためかもしれない。

下記のように client-go は k/api、k/apimachinery に依存しているように見える。

 21 import (
 22         v1beta1 "k8s.io/api/extensions/v1beta1"
 23         v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 24         types "k8s.io/apimachinery/pkg/types"
 25         watch "k8s.io/apimachinery/pkg/watch"
 26         scheme "k8s.io/client-go/kubernetes/scheme"
 27         rest "k8s.io/client-go/rest"
 28 )

k/api、k/apimachinery での依存箇所を特定し、それぞれで行うべき追加コードを特定する。 k/api 依存している k/client-go のコード

 43         Get(name string, options v1.GetOptions) (*v1beta1.Deployment, error)

API のオブジェクト定義として参照

k/apimachinery 依存している k/client-go のコード

oomichi commented 6 years ago

kubernetesコミュニティでのテスト追加に関する議論 oomichi

@hh hi, I start improving the test coverage based on apisnoop result, thanks for your work on that.
Do we have a common procedure/process for improving the coverage?
Today I just picked one API ("GET /apis/extensions") from apisnoop result and submitted it as https://github.com/kubernetes/kubernetes/issues/66522 as first step.

hh

@oomichi I'd think sig-api-machinery would  curate /apis/extensions, you should reach out to them for input regarding what tests would make sense for them.
As far as a process, talking with the SIGs that curate specific areas of low coverage is definitely the first steps.

oomichi

@hh that makes sense, thanks. The corresponding subcommand of kubectl also is not implemented today.
So it is nice to discuss it also in sig-api-machinery, will try

hh

Identifying e2e tests that are not-flakey and suggesting to sig-architecture for upgrade to conformance is probably next.
Though it may make sense to help/suggest the SIGs themselves get involved in the recommendations