Skip to content
18 changes: 16 additions & 2 deletions cmd/func-util/main.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New added logic with conditions without tests covering it.

I noticed that tests are missing for main.go and s2i_generate.go. Please add them in a follow up PR. Or is there a reason not to add them?

Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"knative.dev/func/pkg/tar"
)

const middlewareFileName = "middleware-version"

func main() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand Down Expand Up @@ -93,13 +95,18 @@ func scaffold(ctx context.Context) error {

middlewareVersion, err := scaffolding.MiddlewareVersion(f.Root, f.Runtime, f.Invoke, embeddedRepo.FS())
if err != nil {
return fmt.Errorf("cannot get middleware version: %w", err)
_, _ = fmt.Fprintf(os.Stderr, "warning: cannot get middleware version: %v\n", err)
middlewareVersion = "<unknown>"
}

if err := os.WriteFile("/tekton/results/middlewareVersion", []byte(middlewareVersion), 0644); err != nil {
return fmt.Errorf("cannot write middleware version as a result: %w", err)
}

if err := os.WriteFile(middlewareFileName, []byte(middlewareVersion), 0644); err != nil {
return fmt.Errorf("cannot write middleware version as a file: %w", err)
}

if f.Runtime != "go" && f.Runtime != "python" {
// Scaffolding is for now supported/needed only for Go/Python.
return nil
Expand Down Expand Up @@ -142,6 +149,7 @@ func s2iCmd(ctx context.Context) error {
}

func deploy(ctx context.Context) error {
const imageDigestFileName = "image-digest"
var err error
deployer := knative.NewDeployer(
knative.WithDeployerVerbose(true),
Expand All @@ -161,8 +169,14 @@ func deploy(ctx context.Context) error {
if err != nil {
return fmt.Errorf("cannot load function: %w", err)
}

var digestPart string
if d, err := os.ReadFile(imageDigestFileName); err == nil {
digestPart = "@" + string(d)
}

if len(os.Args) > 2 {
f.Deploy.Image = os.Args[2]
f.Deploy.Image = os.Args[2] + digestPart
}
if f.Deploy.Image == "" {
f.Deploy.Image = f.Image
Expand Down
9 changes: 9 additions & 0 deletions cmd/func-util/s2i_generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ func newS2IGenerateCmd() *cobra.Command {
genCmd := &cobra.Command{
RunE: func(cmd *cobra.Command, args []string) error {
config.envVars = args

if config.middlewareVersion == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing unit (or integration in that case because of direct use of os.ReadFile ) tests to cover this success/error path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll can add more test later. Integration tests for on cluster build should partly cover this already.

bs, err := os.ReadFile(middlewareFileName)
if err != nil {
return fmt.Errorf("cannot read middleware file: %w", err)
}
config.middlewareVersion = string(bs)
}

return runS2IGenerate(cmd.Context(), config)
},
}
Expand Down
12 changes: 0 additions & 12 deletions hack/cluster.sh
Original file line number Diff line number Diff line change
Expand Up @@ -500,18 +500,6 @@ tekton() {

$KUBECTL create clusterrolebinding "${namespace}:knative-serving-namespaced-admin" --clusterrole=knative-serving-namespaced-admin --serviceaccount="${namespace}:default"

# TEMPORARY WORKAROUND: Disable affinity assistant to prevent pod scheduling issues
# This is a workaround for issues where affinity assistant pod names don't match
# what's expected by task pods, causing them to fail scheduling.
# Related issues:
# - https://github.com/tektoncd/pipeline/issues/6740
# - https://github.com/tektoncd/pipeline/issues/7503
# TODO: Remove this workaround once the underlying Tekton issue is resolved
echo "${blue}- Disabling affinity assistant (temporary workaround)${reset}"
$KUBECTL patch configmap feature-flags -n tekton-pipelines \
-p '{"data":{"disable-affinity-assistant":"true", "coschedule":"disabled"}}' \
--type=merge

echo "${green}✅ Tekton${reset}"
}

Expand Down
56 changes: 53 additions & 3 deletions pkg/pipelines/tekton/gitlab_int_test.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func usingNamespace(t *testing.T) string { is missing t.Helper()

Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (

pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
v1 "k8s.io/api/core/v1"
rbacV1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/uuid"
"knative.dev/pkg/apis"
Expand Down Expand Up @@ -112,7 +113,7 @@ func TestInt_Gitlab(t *testing.T) {
t.Fatal(err)
}

err = os.WriteFile(filepath.Join(projDir, "Procfile"), []byte("web: non-existent-app\n"), 0644)
err = os.WriteFile(filepath.Join(projDir, "main.go"), []byte(mainGo), 0644)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -173,13 +174,32 @@ func TestInt_Gitlab(t *testing.T) {
case <-buildDoneCh:
t.Log("build done on time")
case <-time.After(time.Minute * 10):
t.Error("build has not been done in time (15 minute timeout)")
t.Error("build has not been done in time (10 minute timeout)")
case <-ctx.Done():
t.Error("cancelled")
}

}

const mainGo = `package main

import "net/http"

func main() {
s := http.Server{
Handler: http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
writer.WriteHeader(200)
_, _ = writer.Write([]byte("OK"))
}),
Addr: ":8080",
}
err := s.ListenAndServe()
if err != nil {
panic(err)
}
}
`

func parseEnv(t *testing.T) (gitlabHostname string, gitlabRootPassword string, pacCtrHostname string, err error) {
if enabled, _ := strconv.ParseBool(os.Getenv("FUNC_INT_GITLAB_ENABLED")); !enabled {
t.Skip("GitLab tests are disabled")
Expand Down Expand Up @@ -615,9 +635,39 @@ func usingNamespace(t *testing.T) string {
t.Fatal(err)
}
t.Cleanup(func() {
deleteOpts := metav1.DeleteOptions{}
pp := metav1.DeletePropagationForeground
deleteOpts := metav1.DeleteOptions{
PropagationPolicy: &pp,
}
_ = k8sClient.CoreV1().Namespaces().Delete(context.Background(), name, deleteOpts)
})

crbName := name + ":knative-serving-namespaced-admin"
crb := &rbacV1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: crbName,
},
Subjects: []rbacV1.Subject{
{
Kind: "ServiceAccount",
Name: "default",
Namespace: name,
},
},
RoleRef: rbacV1.RoleRef{
Name: "knative-serving-namespaced-admin",
Kind: "ClusterRole",
APIGroup: "rbac.authorization.k8s.io",
},
}
_, err = k8sClient.RbacV1().ClusterRoleBindings().Create(context.Background(), crb, metav1.CreateOptions{})
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() {
_ = k8sClient.RbacV1().ClusterRoleBindings().Delete(context.Background(), crbName, metav1.DeleteOptions{})
})

return name
}

Expand Down
Loading
Loading