KUBESAW-230: add pairing mechanism #135
Conversation
rsoaresd
requested review from
MatousJobanek,
alexeykazakov,
drpaneas,
fbm3307,
jrosental,
metlos,
mfrancisc,
rajivnathan,
ranakan19 and
xcoulon
as code owners
alexeykazakov
left a comment
alexeykazakov
left a comment
There was a problem hiding this comment.
So less bach scripts and more go code! Nice :)
rsoaresd
force-pushed
the
add_pairing_go_script
branch
from
9401970 to
42de922
Compare
MatousJobanek
left a comment
MatousJobanek
left a comment
There was a problem hiding this comment.
Thanks a lot for transforming the script into the go code.
Apart from the comments, I would also move the go project out of the scripts folder - just add a folder "pairing" in the root of the project.
scripts/ci/pairing/pkg/cmd/pair.go
Outdated
| // the pairing can be triggered by CI job or GH action | ||
| func getCurrentPRInfo() (*PullRequestMetadata, error) { | ||
| pr := &PullRequestMetadata{} | ||
| jobSpecEnvVarData := os.Getenv("JOB_SPEC") |
There was a problem hiding this comment.
can you add a comment what the JOB_SPEC variable contains? Is it something similar to CLONEREFS_OPTIONS?
There was a problem hiding this comment.
Sure! The info that we need can also be got from JOB_SPEC:
{
"type": "presubmit",
"job": "pull-ci-codeready-toolchain-toolchain-e2e-master-e2e",
"buildid": "1889023022812106752",
"prowjobid": "9ccb229f-aebf-45d6-90e2-2388663e8b9a",
"refs": {
"org": "codeready-toolchain",
"repo": "toolchain-e2e",
"repo_link": "https://github.com/codeready-toolchain/toolchain-e2e",
"base_ref": "master",
"base_sha": "47ac08434063871caf78c8f3d6dbab6df61ecb63",
"base_link": "https://github.com/codeready-toolchain/toolchain-e2e/commit/47ac08434063871caf78c8f3d6dbab6df61ecb63",
"pulls": [
{
"number": 1113,
"author": "rsoaresd",
"sha": "67ece9d9716bcc8556f91c0c909ecea4b7c17bff",
"head_ref": "test-pairing",
"link": "https://github.com/codeready-toolchain/toolchain-e2e/pull/1113",
"commit_link": "https://github.com/codeready-toolchain/toolchain-e2e/pull/1113/commits/67ece9d9716bcc8556f91c0c909ecea4b7c17bff",
"author_link": "https://github.com/rsoaresd"
}
]
},
"decoration_config": {
"timeout": "2h0m0s",
"grace_period": "15s",
"utility_images": {
"clonerefs": "us-docker.pkg.dev/k8s-infra-prow/images/clonerefs:v20250205-e871edfd1",
"initupload": "us-docker.pkg.dev/k8s-infra-prow/images/initupload:v20250205-e871edfd1",
"entrypoint": "us-docker.pkg.dev/k8s-infra-prow/images/entrypoint:v20250205-e871edfd1",
"sidecar": "us-docker.pkg.dev/k8s-infra-prow/images/sidecar:v20250205-e871edfd1"
},
"resources": {
"clonerefs": {
"limits": {
"memory": "3Gi"
},
"requests": {
"cpu": "100m",
"memory": "500Mi"
}
},
"initupload": {
"limits": {
"memory": "200Mi"
},
"requests": {
"cpu": "100m",
"memory": "50Mi"
}
},
"place_entrypoint": {
"limits": {
"memory": "100Mi"
},
"requests": {
"cpu": "100m",
"memory": "25Mi"
}
},
"sidecar": {
"limits": {
"memory": "2Gi"
},
"requests": {
"cpu": "100m",
"memory": "250Mi"
}
}
},
"gcs_configuration": {
"bucket": "test-platform-results",
"path_strategy": "single",
"default_org": "openshift",
"default_repo": "origin",
"mediaTypes": {
"log": "text/plain"
},
"compress_file_types": [
"txt",
"log",
"json",
"tar",
"html",
"yaml"
]
},
"gcs_credentials_secret": "gce-sa-credentials-gcs-publisher",
"skip_cloning": true,
"censor_secrets": true
}
}
scripts/ci/pairing/pkg/cmd/pair.go
Outdated
| } | ||
|
|
There was a problem hiding this comment.
if neither OpenShift-CI nor GH actions are recognized, then we should return an error
There was a problem hiding this comment.
I forgot the detail that when neither OpenShift-CI nor GH actions, we need to clone the parent repo
rsoaresd
force-pushed
the
add_pairing_go_script
branch
from
4b1909d to
ed0a81e
Compare
improve improvements improve improve clean improve clean improve improve improve improve
rsoaresd
force-pushed
the
add_pairing_go_script
branch
from
8a583d8 to
b8323b9
Compare
MatousJobanek
left a comment
MatousJobanek
left a comment
There was a problem hiding this comment.
Looks really nice 👍
pairing/pkg/cmd/pair_test.go
Outdated
| t.Setenv("AUTHOR", "rsoaresd") | ||
| t.Setenv("GITHUB_HEAD_REF", "clean_only_when_test_passed") |
There was a problem hiding this comment.
the success of this unit-test relies on the presence of your fork and the branch with this name there.
Let's use more reliable approach - you can mock the call to .../info/refs?service=git-upload-pack - you can use Gock (that we use at other places too) or any other library
| .PHONY: build-pairing | ||
| build-pairing: |
There was a problem hiding this comment.
this looks like a duplicate of the build target in build.mk makefile
|
I also noticed that the PR contains some workload-analyzer submodule - probably added by accident https://github.com/codeready-toolchain/toolchain-cicd/pull/135/files#diff-98b7ead5e589543bf9211348ee50cd62c0bedf50539dd66c3d048a4e1af44ec0 |
pairing/pkg/cmd/pair.go
Outdated
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| type PairingServiceInterface interface { |
There was a problem hiding this comment.
If we replaced this interface with a simple func we could get rid of both MockPairingService and PairingService and just replace them with their respective shouldPair impls.
Additionally, if you could mock out the actual HTTP call in shouldPair func, we could also no longer require passing in the pairing func (or interface) to Pair() altogether and just use the actual impl also in the tests.
rsoaresd
changed the title
4 participants
Description
This PR adds a pairing script to reduce pairing complexity and pairing duplication
Check codeready-toolchain/toolchain-e2e#1113
Note
For host-operator, member-operator, and registration-service, the changes are local for now until this PR and codeready-toolchain/toolchain-e2e#1113 are merged
Issue ticket number and link
KUBESAW-230