From dd07c682013ef84c9cf82dc50dbe2c7b52da995b Mon Sep 17 00:00:00 2001 From: Ta-Ching Chen Date: Sun, 2 Feb 2020 00:39:02 +0800 Subject: [PATCH] Set package initial status if its empty (#1522) If the user applies package YAML file has no status field, the package won't be able to be compiled or deployed due to lack of status. This PR aims to add a check at buildermgr to set initial package status to those packages. --- pkg/apis/core/v1/types.go | 7 ++ pkg/buildermgr/buildermgr.go | 2 +- pkg/buildermgr/pkgwatcher.go | 72 ++++++++++++++++--- pkg/controller/controller.go | 1 - test/test_utils.sh | 3 +- test/tests/test_kubectl/spec-yaml/env-go.yaml | 18 +++++ .../test_kubectl/spec-yaml/function-go.yaml | 47 ++++++++++++ test/tests/test_kubectl/test_kubectl.sh | 50 +++++++++++++ test/utils.sh | 18 +++++ 9 files changed, 204 insertions(+), 14 deletions(-) create mode 100644 test/tests/test_kubectl/spec-yaml/env-go.yaml create mode 100644 test/tests/test_kubectl/spec-yaml/function-go.yaml create mode 100755 test/tests/test_kubectl/test_kubectl.sh diff --git a/pkg/apis/core/v1/types.go b/pkg/apis/core/v1/types.go index 408402fe..0d120b2e 100644 --- a/pkg/apis/core/v1/types.go +++ b/pkg/apis/core/v1/types.go @@ -268,6 +268,9 @@ type ( // PackageStatus contains the build status of a package also the build log for examination. PackageStatus struct { + // TODO: Add another status field to indicate whether a package + // is ready for deploy instead of setting "none" in build status. + // BuildStatus is the package build status. BuildStatus BuildStatus `json:"buildstatus,omitempty"` @@ -684,3 +687,7 @@ type ( GetObjectMeta() metav1.Object } ) + +func (a Archive) IsEmpty() bool { + return len(a.Literal) == 0 && len(a.URL) == 0 +} diff --git a/pkg/buildermgr/buildermgr.go b/pkg/buildermgr/buildermgr.go index f526a6ce..11783991 100644 --- a/pkg/buildermgr/buildermgr.go +++ b/pkg/buildermgr/buildermgr.go @@ -48,7 +48,7 @@ func Start(logger *zap.Logger, storageSvcUrl string, envBuilderNamespace string) pkgWatcher := makePackageWatcher(bmLogger, fissionClient, kubernetesClient, envBuilderNamespace, storageSvcUrl) - go pkgWatcher.watchPackages(fissionClient, kubernetesClient, envBuilderNamespace) + go pkgWatcher.watchPackages() select {} } diff --git a/pkg/buildermgr/pkgwatcher.go b/pkg/buildermgr/pkgwatcher.go index 84fa80dc..e451ad77 100644 --- a/pkg/buildermgr/pkgwatcher.go +++ b/pkg/buildermgr/pkgwatcher.go @@ -75,12 +75,6 @@ func makePackageWatcher(logger *zap.Logger, fissionClient *crd.FissionClient, k8 // 6. Update package status to succeed state // *. Update package status to failed state,if any one of steps above failed/time out func (pkgw *packageWatcher) build(buildCache *cache.Cache, srcpkg *fv1.Package) { - - // Ignore non-pending state packages. - if srcpkg.Status.BuildStatus != fv1.BuildStatusPending { - return - } - // Ignore duplicate build requests key := fmt.Sprintf("%v-%v", srcpkg.ObjectMeta.Name, srcpkg.ObjectMeta.ResourceVersion) _, err := buildCache.Set(key, srcpkg) @@ -228,20 +222,76 @@ func (pkgw *packageWatcher) build(buildCache *cache.Cache, srcpkg *fv1.Package) zap.String("package", fmt.Sprintf("%s.%s", pkg.ObjectMeta.Name, pkg.ObjectMeta.Namespace))) } -func (pkgw *packageWatcher) watchPackages(fissionClient *crd.FissionClient, - kubernetesClient *kubernetes.Clientset, builderNamespace string) { +func (pkgw *packageWatcher) watchPackages() { buildCache := cache.MakeCache(0, 0) lw := k8sCache.NewListWatchFromClient(pkgw.fissionClient.CoreV1().RESTClient(), "packages", apiv1.NamespaceAll, fields.Everything()) - pkgStore, controller := k8sCache.NewInformer(lw, &fv1.Package{}, 60*time.Second, k8sCache.ResourceEventHandlerFuncs{ + + processPkg := func(pkg *fv1.Package) { + var err error + + if len(pkg.Status.BuildStatus) == 0 { + _, err = setInitialBuildStatus(pkgw.fissionClient, pkg) + if err != nil { + pkgw.logger.Error("error filling package status", zap.Error(err)) + } + // once we update the package status, an update event + // will arrive and handle by UpdateFunc later. So we + // don't need to build the package at this moment. + return + } + + // Only build pending state packages. + if pkg.Status.BuildStatus == fv1.BuildStatusPending { + go pkgw.build(buildCache, pkg) + } + } + + pkgStore, controller := k8sCache.NewInformer(lw, &fv1.Package{}, 60*time.Minute, k8sCache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { pkg := obj.(*fv1.Package) - go pkgw.build(buildCache, pkg) + processPkg(pkg) }, UpdateFunc: func(oldObj, newObj interface{}) { + oldPkg := oldObj.(*fv1.Package) pkg := newObj.(*fv1.Package) - go pkgw.build(buildCache, pkg) + + // TODO: Once enable "/status", check generation for spec changed instead. + // Before "/status" is enabled, the generation and resource version will be changed + // if we update the status of a package, hence we are not able to differentiate + // the spec change or status change. So we only build package which's status + // us "pending" and user have to use "kubectl replace" to update a package. + if oldPkg.ResourceVersion == pkg.ResourceVersion && + pkg.Status.BuildStatus != fv1.BuildStatusPending { + return + } + processPkg(pkg) }, }) + pkgw.pkgStore = pkgStore controller.Run(make(chan struct{})) } + +// setInitialBuildStatus sets initial build status to a package if it is empty. +// This normally occurs when the user applies package YAML files that have no status field +// through kubectl. +func setInitialBuildStatus(fissionClient *crd.FissionClient, pkg *fv1.Package) (*fv1.Package, error) { + pkg.Status = fv1.PackageStatus{ + LastUpdateTimestamp: metav1.Time{Time: time.Now().UTC()}, + } + if !pkg.Spec.Deployment.IsEmpty() { + // if the deployment archive is not empty, + // we assume it's a deployable package no matter + // the source archive is empty or not. + pkg.Status.BuildStatus = fv1.BuildStatusNone + } else if !pkg.Spec.Source.IsEmpty() { + pkg.Status.BuildStatus = fv1.BuildStatusPending + } else { + // mark package failed since we cannot do anything with it. + pkg.Status.BuildStatus = fv1.BuildStatusFailed + pkg.Status.BuildLog = "Both deploy and source archive are empty" + } + + // TODO: use UpdateStatus to update status + return fissionClient.CoreV1().Packages(pkg.Namespace).Update(pkg) +} diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 93ece256..429d2b4c 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -18,7 +18,6 @@ package controller import ( "context" - "go.uber.org/zap" "github.com/fission/fission/pkg/crd" diff --git a/test/test_utils.sh b/test/test_utils.sh index f2523393..18fa856f 100755 --- a/test/test_utils.sh +++ b/test/test_utils.sh @@ -541,7 +541,8 @@ run_all_tests() { $ROOT/test/tests/test_environments/test_go_env.sh \ $ROOT/test/tests/mqtrigger/nats/test_mqtrigger.sh \ $ROOT/test/tests/mqtrigger/nats/test_mqtrigger_error.sh \ - $ROOT/test/tests/test_huge_response/test_huge_response.sh + $ROOT/test/tests/test_huge_response/test_huge_response.sh \ + $ROOT/test/tests/test_kubectl/test_kubectl.sh FAILURES=$? export JOBS=3 diff --git a/test/tests/test_kubectl/spec-yaml/env-go.yaml b/test/tests/test_kubectl/spec-yaml/env-go.yaml new file mode 100644 index 00000000..d0989f71 --- /dev/null +++ b/test/tests/test_kubectl/spec-yaml/env-go.yaml @@ -0,0 +1,18 @@ +apiVersion: fission.io/v1 +kind: Environment +metadata: + creationTimestamp: null + name: go-spec-kubectl + namespace: default +spec: + builder: + command: build + image: fission/go-builder-1.12:1.7.1 + imagepullsecret: "" + keeparchive: false + poolsize: 3 + resources: {} + runtime: + image: fission/go-env-1.12:1.7.1 + terminationGracePeriod: 5 + version: 2 diff --git a/test/tests/test_kubectl/spec-yaml/function-go.yaml b/test/tests/test_kubectl/spec-yaml/function-go.yaml new file mode 100644 index 00000000..edc1ead7 --- /dev/null +++ b/test/tests/test_kubectl/spec-yaml/function-go.yaml @@ -0,0 +1,47 @@ +apiVersion: fission.io/v1 +kind: Package +metadata: + creationTimestamp: null + name: go-b4bbb0e0-2d93-47f0-8c4e-eea644eec2a9 + namespace: default +spec: + deployment: + checksum: {} + environment: + name: go-spec-kubectl + namespace: default + source: + checksum: + sum: aa595bb952047c517d849f8fc9e490fdabc37d83795392074a0b15a59748004f + type: sha256 + type: url + url: https://raw.githubusercontent.com/fission/fission/master/examples/go/hello.gogo # this is intentional + +--- +apiVersion: fission.io/v1 +kind: Function +metadata: + creationTimestamp: null + name: go-spec-kubectl + namespace: default +spec: + InvokeStrategy: + ExecutionStrategy: + ExecutorType: poolmgr + MaxScale: 0 + MinScale: 0 + SpecializationTimeout: 120 + TargetCPUPercent: 0 + StrategyType: execution + configmaps: null + environment: + name: go-spec-kubectl + namespace: default + functionTimeout: 60 + package: + functionName: Handler + packageref: + name: go-b4bbb0e0-2d93-47f0-8c4e-eea644eec2a9 + namespace: default + resources: {} + secrets: null diff --git a/test/tests/test_kubectl/test_kubectl.sh b/test/tests/test_kubectl/test_kubectl.sh new file mode 100755 index 00000000..b13119df --- /dev/null +++ b/test/tests/test_kubectl/test_kubectl.sh @@ -0,0 +1,50 @@ +#!/bin/bash + +set -euo pipefail +source $(dirname $0)/../../utils.sh + +TEST_ID=$(generate_test_id) +echo "TEST_ID = $TEST_ID" + +tmp_dir="/tmp/test-$TEST_ID" +mkdir -p $tmp_dir + +ROOT=$(dirname $0)/../../.. + +cd $ROOT/test/tests/test_kubectl + +cleanup() { + kubectl delete -f spec-yaml -R || true +} + +if [ -z "${TEST_NOCLEANUP:-}" ]; then + trap cleanup EXIT +else + log "TEST_NOCLEANUP is set; not cleaning up test artifacts afterwards." +fi + +name="go-spec-kubectl" +pkgName="go-b4bbb0e0-2d93-47f0-8c4e-eea644eec2a9" + +# cleanup first +cleanup + +# apply environment & function +kubectl apply -f spec-yaml -R + +# wait for build to finish +timeout 90 bash -c "wait_for_builder $name" +timeout 90 bash -c "waitBuildExpectedStatus $pkgName failed" + +sed -i 's/gogo/go/g' spec-yaml/function-go.yaml + +# before we enable "/status" this should be failed. +kubectl apply -f spec-yaml/function-go.yaml +timeout 90 bash -c "waitBuildExpectedStatus $pkgName failed" + +kubectl replace -f spec-yaml/function-go.yaml +timeout 90 bash -c "waitBuild $pkgName" + +fission fn test --name $name + +log "Test PASSED" diff --git a/test/utils.sh b/test/utils.sh index 6bb5cf2b..828d4bff 100755 --- a/test/utils.sh +++ b/test/utils.sh @@ -152,6 +152,24 @@ waitBuild() { } export -f waitBuild +waitBuildExpectedStatus() { + pkg=$1 + status=$2 + + log "Waiting for builder manager to finish the build with status $status" + + set +e + while true; do + kubectl --namespace default get packages $pkg -o jsonpath='{.status.buildstatus}'|grep $status + if [[ $? -eq 0 ]]; then + break + fi + sleep 1 + done + set -e +} +export -f waitBuildExpectedStatus + ## Common env parameters export FISSION_NAMESPACE=${FISSION_NAMESPACE:-fission}