openyurtio / yurt-app-manager

The workload controller manager from NodePool level in OpenYurt cluster
Apache License 2.0
6 stars 1 forks source link

[BUG] YurtAppSet can't create StatefulSet workload #141

Open SQxiaoxiaomeng opened 1 year ago

SQxiaoxiaomeng commented 1 year ago

What happened: When I create a StatefulSet type YurtAppSet As follows,

apiVersion: apps.openyurt.io/v1alpha1
kind: YurtAppSet
metadata:
  labels:
    controller-tools.k8s.io: "1.0"
  name: yas-sts-test
spec:
  selector:
    matchLabels:
      app: yas-sts-test
  workloadTemplate:
    statefulSetTemplate:
      metadata:
        labels:
          app: yas-sts-test
      spec:
        podManagementPolicy: OrderedReady
        replicas: 1
        revisionHistoryLimit: 10
        selector:
        matchLabels:
          app: yas-sts-test
        serviceName: test-sts
        template:
          metadata:
            labels:
              app: yas-sts-test
          spec:
            containers:
              - name: yas-sts-test
                image: nginx
  topology:
    pools:
    - name: beijing
      nodeSelectorTerm:
        matchExpressions:
        - key: apps.openyurt.io/nodepool
          operator: In
          values:
          - beijing
      replicas: 1
  revisionHistoryLimit: 5

I get an error, Internal error occurred: failed calling webhook "vyurtappset.kb.io": Post https://yurt-app-manager-webhook.default.svc:443/validate-apps-openyurt-io-v1alpha1-yurtappset?timeout=10s: EOF And I also get a panic error in Yurt-App-Manager pod:

2023-02-24T08:00:11.504Z        DEBUG   controller-runtime.webhook.webhooks     received request        {"webhook": "/validate-apps-openyurt-io-v1alpha1-yurtappset", "UID": "6386081a-f772-4cf9-916f-1f1ba4094794", "kind": "apps.openyurt.io/v1alpha1, Kind=YurtAppSet", "resource": {"group":"apps.openyurt.io","version":"v1alpha1","resource":"yurtappsets"}}
2023/02/24 08:00:11 http: panic serving 10.113.65.98:46336: runtime error: invalid memory address or nil pointer dereference
goroutine 498 [running]:
net/http.(*conn).serve.func1()
        /usr/local/go/src/net/http/server.go:1801 +0xb9
panic({0x173b3e0, 0x28aa4e0})
        /usr/local/go/src/runtime/panic.go:1047 +0x266
github.com/openyurtio/yurt-app-manager/pkg/yurtappmanager/webhook/yurtappset.validateYurtAppSetSpec({0x1941d48, 0x4}, 0xc00082c118, 0x0)
        /build/pkg/yurtappmanager/webhook/yurtappset/yurtappset_validation.go:56 +0x3dd
github.com/openyurtio/yurt-app-manager/pkg/yurtappmanager/webhook/yurtappset.validateYurtAppSet({0x1bafb50, 0xc0003e8c80}, 0xc00082c000)
        /build/pkg/yurtappmanager/webhook/yurtappset/yurtappset_validation.go:113 +0xb2
github.com/openyurtio/yurt-app-manager/pkg/yurtappmanager/webhook/yurtappset.(*YurtAppSetHandler).ValidateCreate(0xc000d94ea0, {0x24, 0xc000d750b0}, {0x1b6bc08, 0xc00082c000})
        /build/pkg/yurtappmanager/webhook/yurtappset/yurtappset_webhook.go:80 +0x136
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*validatorForType).Handle(_, {_, _}, {{{0xc000d94ea0, 0x24}, {{0xc000d750b0, 0x10}, {0xc000d750c0, 0x8}, {0xc000d750d0, ...}}, ...}})
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/webhook/admission/validator_custom.go:77 +0x258
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).Handle(_, {_, _}, {{{0xc000d94ea0, 0x24}, {{0xc000d750b0, 0x10}, {0xc000d750c0, 0x8}, {0xc000d750d0, ...}}, ...}})
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/webhook/admission/webhook.go:146 +0xa2
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).ServeHTTP(0xc000728570, {0x7f846321bf08, 0xc0002f5130}, 0xc0008aca00)
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/webhook/admission/http.go:99 +0xef0
github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerInFlight.func1({0x7f846321bf08, 0xc0002f5130}, 0xc0002f5130)
        /go/pkg/mod/github.com/prometheus/client_golang@v1.11.0/prometheus/promhttp/instrument_server.go:40 +0xd4
net/http.HandlerFunc.ServeHTTP(0x1b7c240, {0x7f846321bf08, 0xc0002f5130}, 0x7f848a1295b8)
        /usr/local/go/src/net/http/server.go:2046 +0x2f
github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1({0x1b7c240, 0xc0001828c0}, 0xc0008aca00)
        /go/pkg/mod/github.com/prometheus/client_golang@v1.11.0/prometheus/promhttp/instrument_server.go:101 +0x92
net/http.HandlerFunc.ServeHTTP(0xc00070cd00, {0x1b7c240, 0xc0001828c0}, 0xc000cf6000)
        /usr/local/go/src/net/http/server.go:2046 +0x2f
github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func2({0x1b7c240, 0xc0001828c0}, 0xc0008aca00)
        /go/pkg/mod/github.com/prometheus/client_golang@v1.11.0/prometheus/promhttp/instrument_server.go:76 +0xa2
net/http.HandlerFunc.ServeHTTP(0x1689320, {0x1b7c240, 0xc0001828c0}, 0xc0001828c0)
        /usr/local/go/src/net/http/server.go:2046 +0x2f
net/http.(*ServeMux).ServeHTTP(0xc000c2ca84, {0x1b7c240, 0xc0001828c0}, 0xc0008aca00)
        /usr/local/go/src/net/http/server.go:2424 +0x149
net/http.serverHandler.ServeHTTP({0x1b6ff88}, {0x1b7c240, 0xc0001828c0}, 0xc0008aca00)
        /usr/local/go/src/net/http/server.go:2878 +0x43b
net/http.(*conn).serve(0xc000475860, {0x1b81218, 0xc000810360})
        /usr/local/go/src/net/http/server.go:1929 +0xb08
created by net/http.(*Server).Serve
        /usr/local/go/src/net/http/server.go:3033 +0x4e8

What you expected to happen: crate YurtAppSet successful

How to reproduce it (as minimally and precisely as possible):

cat <<EOF | kubectl create -f - 
apiVersion: apps.openyurt.io/v1alpha1
kind: YurtAppSet
metadata:
  labels:
    controller-tools.k8s.io: "1.0"
  name: yas-sts-test
spec:
  selector:
    matchLabels:
      app: yas-sts-test
  workloadTemplate:
    statefulSetTemplate:
      metadata:
        labels:
          app: yas-sts-test
      spec:
        podManagementPolicy: OrderedReady
        replicas: 1
        revisionHistoryLimit: 10
        selector:
        matchLabels:
          app: yas-sts-test
        serviceName: test-sts
        template:
          metadata:
            labels:
              app: yas-sts-test
          spec:
            containers:
              - name: yas-sts-test
                image: nginx
  topology:
    pools:
    - name: beijing
      nodeSelectorTerm:
        matchExpressions:
        - key: apps.openyurt.io/nodepool
          operator: In
          values:
          - beijing
      replicas: 1
  revisionHistoryLimit: 5
<<EOF

Anything else we need to know?:

Environment:

CENTOS_MANTISBT_PROJECT="CentOS-7" CENTOS_MANTISBT_PROJECT_VERSION="7" REDHAT_SUPPORT_PRODUCT="centos" REDHAT_SUPPORT_PRODUCT_VERSION="7"

others

/kind bug

SQxiaoxiaomeng commented 1 year ago

I checked the source code of yurt-app-manager and found that there is a bug in the webhook verification:

pkg/yurtappmanager/webhook/yurtappset/yurtappset_validation.go:44

func validateYurtAppSetSpec(c client.Client, spec *unitv1alpha1.YurtAppSetSpec, fldPath *field.Path) field.ErrorList {
    allErrs := field.ErrorList{}

    if spec.Selector == nil {
        allErrs = append(allErrs, field.Required(fldPath.Child("selector"), ""))
    } else {
        allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
        if len(spec.Selector.MatchLabels)+len(spec.Selector.MatchExpressions) == 0 {
            allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "empty selector is invalid for statefulset"))
        }
    }

    klog.Infof("sel:%v, label: %v\n", spec.Selector, spec.WorkloadTemplate.DeploymentTemplate.Labels)
    klog.Infof("templatePath:%s", fldPath.Child("workloadTemplate").String())

    selector, err := metav1.LabelSelectorAsSelector(spec.Selector)

There is a bug in klog.Infof("sel:%v, label: %v\n", spec.Selector, spec.WorkloadTemplate.DeploymentTemplate.Labels)

in this case, DeploymentTemplate is nil

kadisi commented 1 year ago

I checked the source code of yurt-app-manager and found that there is a bug in the webhook verification:

pkg/yurtappmanager/webhook/yurtappset/yurtappset_validation.go:44

func validateYurtAppSetSpec(c client.Client, spec *unitv1alpha1.YurtAppSetSpec, fldPath *field.Path) field.ErrorList {
  allErrs := field.ErrorList{}

  if spec.Selector == nil {
      allErrs = append(allErrs, field.Required(fldPath.Child("selector"), ""))
  } else {
      allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
      if len(spec.Selector.MatchLabels)+len(spec.Selector.MatchExpressions) == 0 {
          allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "empty selector is invalid for statefulset"))
      }
  }

  klog.Infof("sel:%v, label: %v\n", spec.Selector, spec.WorkloadTemplate.DeploymentTemplate.Labels)
  klog.Infof("templatePath:%s", fldPath.Child("workloadTemplate").String())

  selector, err := metav1.LabelSelectorAsSelector(spec.Selector)

There is a bug in klog.Infof("sel:%v, label: %v\n", spec.Selector, spec.WorkloadTemplate.DeploymentTemplate.Labels)

in this case, DeploymentTemplate is nil

@SQxiaoxiaomeng yes, this is a bug , we need change DeploymentTemplate to StatefulsetTemplate