This is an automated email from the ASF dual-hosted git repository.
wilfred-s pushed a commit to branch branch-1.9
in repository https://gitbox.apache.org/repos/asf/yunikorn-release.git
The following commit(s) were added to refs/heads/branch-1.9 by this push:
new f422f6b [YUNIKORN-3194] Update PR template to ask about AI
contribution (#227)
f422f6b is described below
commit f422f6b3d5373930276a7f68c13aaa4fe3e46bdb
Author: Wilfred Spiegelenburg <[email protected]>
AuthorDate: Fri Jun 19 15:56:39 2026 +1000
[YUNIKORN-3194] Update PR template to ask about AI contribution (#227)
Update PR template:
* ask about AI contribution
* Update Jira area
* Check for testing performed (new make target to simplify task)
Cleanup of Makefile: variables for tools, build and staging dirs
New targets for helm lint
Remove unused targets
Fixed lint errors to get a clean build run.
Closes: #227
Signed-off-by: Wilfred Spiegelenburg <[email protected]>
(cherry picked from commit 73498a6b2b36028ac689341763432dd974837fa9)
---
.github/PULL_REQUEST_TEMPLATE | 49 ++++++++++++--------
.github/workflows/pre-commit.yml | 2 +-
.gitignore | 9 +---
Makefile | 86 +++++++++++++++--------------------
perf-tools/.gitignore | 1 +
perf-tools/framework/app_analyzer.go | 4 +-
perf-tools/framework/app_manager.go | 4 +-
perf-tools/framework/node_analyzer.go | 6 ++-
perf-tools/scenarios/e2e_perf.go | 1 +
perf-tools/scenarios/node_fairness.go | 1 +
release-tools/merge_pr.sh | 14 +++---
11 files changed, 87 insertions(+), 90 deletions(-)
diff --git a/.github/PULL_REQUEST_TEMPLATE b/.github/PULL_REQUEST_TEMPLATE
index 0d56d00..51cee43 100644
--- a/.github/PULL_REQUEST_TEMPLATE
+++ b/.github/PULL_REQUEST_TEMPLATE
@@ -1,28 +1,37 @@
-### What is this PR for?
-A few sentences describing the overall goals of the pull request's commits.
-First time? Check out the contributing guide -
http://yunikorn.apache.org/community/how_to_contribute
+### Description
+Short description of this pull request, can be as simple as the commit message
used.
+First time contributing? Check out the contributing guide:
https://yunikorn.apache.org/community/how_to_contribute
-### What type of PR is it?
-* [ ] - Bug Fix
-* [ ] - Improvement
-* [ ] - Feature
-* [ ] - Documentation
-* [ ] - Hot Fix
-* [ ] - Refactoring
+### Type of change
+Please delete options that are not relevant.
-### Todos
-* [ ] - Task
+- [ ] Bug Fix
+- [ ] Improvement
+- [ ] Feature
+- [ ] Refactoring
+- [ ] Documentation
-### What is the Jira issue?
-* Open an issue on Jira https://issues.apache.org/jira/browse/YUNIKORN/
-* Put link here, and add [YUNIKORN-*Jira number*] in PR title, eg.
`[YUNIKORN-2] Gang scheduling interface parameters`
+### Jira issue
+Jira ID : https://issues.apache.org/jira/browse/YUNIKORN-XXXXX
-### How should this be tested?
+- [ ] I have created a Jira issue for this pull request.
+- [ ] The Jira ID is part of the title of this pull request.
-### Screenshots (if appropriate)
+### AI Tooling
+If an AI tool was used:
+- [ ] The PR includes the phrase "Generated by \<tool>", where \<tool> is the
name of the AI tool used.
+- [ ] My use of AI contributions follows the ASF legal policy.
+
+Check https://www.apache.org/legal/generative-tooling.html for details.
+
+### How has this been tested?
+- [ ] New unit tests were added to cover new or changed code paths.
+- [ ] `make test_all` was run, and no failures reported.
### Questions:
-* [ ] - The licenses files need update.
-* [ ] - There is breaking changes for older versions.
-* [ ] - It needs documentation.
+- [ ] The change needs documentation, a pull request for apache/yunikorn-site
repository will be created.
+- [ ] There is breaking changes for older versions: jira is tagged with
`release-notes` label.
+- [ ] The licenses files needs to be updated.
+
+### Screenshots or other details
diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml
index d3d42f8..9b48650 100644
--- a/.github/workflows/pre-commit.yml
+++ b/.github/workflows/pre-commit.yml
@@ -37,4 +37,4 @@ jobs:
with:
go-version-file: .go_version
- name: Check license
- run: make license-check
+ run: make test_all
diff --git a/.gitignore b/.gitignore
index 4ca6bc6..3355112 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,12 +1,7 @@
.idea
.DS_Store
-staging/
-staging/*
-staging/**
-target
*.tmp
-.build
*.swp
*.a
-/tools/
-/build/
+tools/
+build/
diff --git a/Makefile b/Makefile
index d01ce19..fa22f29 100644
--- a/Makefile
+++ b/Makefile
@@ -15,6 +15,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+.PHONY: lint check_scripts license-check helm_lint
+.PHONY: perf-tools tools clean distclean
+
# Check if this GO tools version used is at least the version of go specified
in
# the go.mod file. The version in go.mod should be in sync with other repos.
@@ -41,7 +44,8 @@ endif
# Make sure we are in the same directory as the Makefile
BASE_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
-TOOLS_DIR=tools
+TOOLS_DIR := tools
+BUILD_DIR := build
# Force Go modules even when checked out inside GOPATH
GO111MODULE := on
@@ -101,15 +105,23 @@ endif
GOLANGCI_LINT_VERSION=2.10.1
GOLANGCI_LINT_PATH=$(TOOLS_DIR)/golangci-lint-v$(GOLANGCI_LINT_VERSION)
GOLANGCI_LINT_BIN=$(GOLANGCI_LINT_PATH)/golangci-lint
-GOLANGCI_LINT_ARCHIVE=golangci-lint-$(GOLANGCI_LINT_VERSION)-$(OS)-$(EXEC_ARCH).tar.gz
GOLANGCI_LINT_ARCHIVEBASE=golangci-lint-$(GOLANGCI_LINT_VERSION)-$(OS)-$(EXEC_ARCH)
+GOLANGCI_LINT_ARCHIVE=$(GOLANGCI_LINT_ARCHIVEBASE).tar.gz
+
+# helm
+HELM_VERSION=v3.12.1
+HELM_PATH=$(TOOLS_DIR)/helm-$(HELM_VERSION)
+HELM_BIN=$(HELM_PATH)/helm
+HELM_ARCHIVE=helm-$(HELM_VERSION)-$(OS)-$(EXEC_ARCH).tar.gz
+HELM_ARCHIVE_BASE=$(OS)-$(EXEC_ARCH)
all:
- $(MAKE) -C $(dir $(BASE_DIR)) build
+ $(MAKE) -C $(dir $(BASE_DIR)) test_all
+
+test_all: license-check check_scripts lint helm_lint
# Install tools
-.PHONY: tools
-tools: $(SHELLCHECK_BIN) $(GOLANGCI_LINT_BIN)
+tools: $(SHELLCHECK_BIN) $(GOLANGCI_LINT_BIN) $(HELM_BIN)
# Install shellcheck
$(SHELLCHECK_BIN):
@@ -118,6 +130,13 @@ $(SHELLCHECK_BIN):
@curl -sSfL
"https://github.com/koalaman/shellcheck/releases/download/$(SHELLCHECK_VERSION)/$(SHELLCHECK_ARCHIVE)"
\
| tar -x -J --strip-components=1 -C "$(SHELLCHECK_PATH)"
"shellcheck-$(SHELLCHECK_VERSION)/shellcheck"
+# Install helm
+$(HELM_BIN):
+ @echo "installing helm $(HELM_VERSION)"
+ @mkdir -p "$(HELM_PATH)"
+ @curl -sSfL "https://get.helm.sh/$(HELM_ARCHIVE)" \
+ | tar -x -z --strip-components=1 -C "$(HELM_PATH)"
"$(HELM_ARCHIVE_BASE)/helm"
+
# Install golangci-lint
$(GOLANGCI_LINT_BIN):
@echo "installing golangci-lint v$(GOLANGCI_LINT_VERSION)"
@@ -125,88 +144,55 @@ $(GOLANGCI_LINT_BIN):
@curl -sSfL
"https://github.com/golangci/golangci-lint/releases/download/v$(GOLANGCI_LINT_VERSION)/$(GOLANGCI_LINT_ARCHIVE)"
\
| tar -x -z --strip-components=1 -C "$(GOLANGCI_LINT_PATH)"
"$(GOLANGCI_LINT_ARCHIVEBASE)/golangci-lint"
-.PHONY: format
-# Run go fmt and goimports
-format:
- @echo "running go fmt"
- @"$(GO)" fmt ./...
- go install golang.org/x/tools/cmd/goimports@latest
- goimports -local
"github.com/apache/yunikorn-core,github.com/apache/yunikorn-scheduler,github.com/apache/yunikorn-release"
-w .
-
-.PHONY: lint
# Run lint against the previous commit for PR and branch build
# In dev setup look at all changes on top of master
lint: $(GOLANGCI_LINT_BIN)
@echo "running golangci-lint"
@"${GOLANGCI_LINT_BIN}" run
+CHART_DIR := helm-charts/yunikorn
+helm_lint: $(HELM_BIN)
+ @echo "running helm lint"
+ @"$(HELM_BIN)" lint "${CHART_DIR}" -f "${CHART_DIR}/values.yaml"
+
# Check scripts
-.PHONY: check_scripts
-ALLSCRIPTS := $(shell find . -not \( -path ./tools -prune \) -not \( -path
./build -prune \) -name '*.sh')
+ALLSCRIPTS := $(shell find . -not \( -path ./"${TOOLS_DIR}" -prune \) -not \(
-path ./"${BUILD_DIR}" -prune \) -name '*.sh')
check_scripts: $(SHELLCHECK_BIN)
@echo "running shellcheck"
@"$(SHELLCHECK_BIN)" ${ALLSCRIPTS}
-.PHONY: license-check
# This is a bit convoluted but using a recursive grep on linux fails to write
anything when run
# from the Makefile. That caused the pull-request license check run from the
github action to
# always pass. The syntax for find is slightly different too but that at least
works in a similar
# way on both Mac and Linux. Excluding all .git* files from the checks.
+LICENSE_CHECK_OUT := $(BUILD_DIR)/license-check.txt
license-check:
@echo "checking license headers:"
ifeq (darwin,$(OS))
- $(shell mkdir -p build && find -E . -not \( -path './.git*' -prune \)
-not \( -path ./build -prune \) -not \( -path ./tools -prune \) -regex
".*\.(go|sh|md|yaml|yml|mod)" -exec grep -L "Licensed to the Apache Software
Foundation" {} \; > build/license-check.txt)
+ $(shell mkdir -p "${BUILD_DIR}" && find -E . -not \( -path './.git*'
-prune \) -not \( -path ./"${BUILD_DIR}" -prune \) -not \( -path
./"${TOOLS_DIR}" -prune \) -regex ".*\.(go|sh|md|yaml|yml|mod)" -exec grep -L
"Licensed to the Apache Software Foundation" {} \; > "${LICENSE_CHECK_OUT}")
else
- $(shell mkdir -p build && find . -not \( -path './.git*' -prune \) -not
\( -path ./build -prune \) -not \( -path ./tools -prune \) -regex
".*\.\(go\|sh\|md\|yaml\|yml\|mod\)" -exec grep -L "Licensed to the Apache
Software Foundation" {} \; > build/license-check.txt)
+ $(shell mkdir -p "${BUILD_DIR}" && find . -not \( -path './.git*'
-prune \) -not \( -path ./"${BUILD_DIR}" -prune \) -not \( -path
./"${TOOLS_DIR}" -prune \) -regex ".*\.\(go\|sh\|md\|yaml\|yml\|mod\)" -exec
grep -L "Licensed to the Apache Software Foundation" {} \; >
"${LICENSE_CHECK_OUT}")
endif
- @if [ -s "build/license-check.txt" ]; then \
+ @if [ -s "${LICENSE_CHECK_OUT}" ]; then \
echo "following files are missing license header:" ; \
- cat build/license-check.txt ; \
+ cat "${LICENSE_CHECK_OUT}" ; \
exit 1; \
fi
@echo " all OK"
-.PHONY: perf-tools
perf-tools:
@echo "Running perf-tools"
@cd perf-tools && make build
@cd ../
-# Check that we use pseudo versions in master
-.PHONY: pseudo
-BRANCH := $(shell git branch --show-current)
-SI_REF := $(shell "$(GO)" list -m -f '{{ .Version }}'
github.com/apache/yunikorn-scheduler-interface)
-CORE_REF := $(shell "$(GO)" list -m -f '{{ .Version }}'
github.com/apache/yunikorn-core)
-
-SI_MATCH := $(shell expr "${SI_REF}" : "v0.0.0-")
-CORE_MATCH := $(shell expr "${CORE_REF}" : "v0.0.0-")
-
-
-pseudo:
- @echo "pseudo version check"
- @if [ "${BRANCH}" = "master" ]; then \
- if [ $(SI_MATCH) -ne 7 ] || [ $(CORE_MATCH) -ne 7 ]; then \
- echo "YuniKorn references MUST all be pseudo versions:"
; \
- echo " SI ref: ${SI_REF}" ; \
- echo " Core ref: ${CORE_REF}" ; \
- exit 1 ; \
- fi ; \
- fi
- @echo " all OK"
-
-# Build the example binaries for dev and test
-
-
# Remove generated build artifacts
-.PHONY: clean
clean:
@echo "cleaning up caches and output"
"$(GO)" clean -cache -testcache -r
@echo "removing generated files"
- @rm -rf build
+ @rm -rf "${BUILD_DIR}"
# Remove all generated content
-.PHONY: distclean
distclean: clean
@echo "removing tools"
@rm -rf "${TOOLS_DIR}"
diff --git a/perf-tools/.gitignore b/perf-tools/.gitignore
new file mode 100644
index 0000000..2f7896d
--- /dev/null
+++ b/perf-tools/.gitignore
@@ -0,0 +1 @@
+target/
diff --git a/perf-tools/framework/app_analyzer.go
b/perf-tools/framework/app_analyzer.go
index 36d2745..f37482a 100644
--- a/perf-tools/framework/app_analyzer.go
+++ b/perf-tools/framework/app_analyzer.go
@@ -121,10 +121,10 @@ func (aa *AppAnalyzer) GetTasksProfiling()
profiling.Profiling {
for _, ts := range aa.appInfo.TasksStatus {
prof.StartExecutionWithTime(ts.CreateTime)
for _, cond := range ts.Conditions {
- prof.AddCheckpointWithTime(string(cond.CondType),
cond.TransitionTime)
+ _ = prof.AddCheckpointWithTime(string(cond.CondType),
cond.TransitionTime) //nolint:errcheck
}
// lastCondTime :=
ts.Conditions[len(ts.Conditions)-1].TransitionTime
- prof.FinishExecutionWithTime(true, endTime)
+ _ = prof.FinishExecutionWithTime(true, endTime)
//nolint:errcheck
}
return prof
}
diff --git a/perf-tools/framework/app_manager.go
b/perf-tools/framework/app_manager.go
index b74b4e7..6cb344c 100644
--- a/perf-tools/framework/app_manager.go
+++ b/perf-tools/framework/app_manager.go
@@ -53,10 +53,10 @@ type DeploymentsAppManager struct {
}
func NewDeploymentsAppManager(kubeClient *utils.KubeClient) AppManager {
- regexp, _ := regexp.Compile(`[_\W]`)
+ regex := regexp.MustCompile(`[_\W]`)
return &DeploymentsAppManager{
kubeClient: kubeClient,
- nameRegexp: regexp,
+ nameRegexp: regex,
}
}
diff --git a/perf-tools/framework/node_analyzer.go
b/perf-tools/framework/node_analyzer.go
index 100a22b..c0d984a 100644
--- a/perf-tools/framework/node_analyzer.go
+++ b/perf-tools/framework/node_analyzer.go
@@ -77,7 +77,11 @@ func (na *NodeAnalyzer) InitNodeInfosBeforeTesting() error {
// so this should be called only if necessary!
func (na *NodeAnalyzer) CalculateAllocatedResource() {
utils.Logger.Info("start loading all pods for calculating allocated
resource")
- podList, _ := na.kubeClient.GetPods("",
utils.GetEverythingListOptions())
+ podList, err := na.kubeClient.GetPods("",
utils.GetEverythingListOptions())
+ if err != nil {
+ utils.Logger.Error("GetPods error", zap.Error(err))
+ return
+ }
utils.Logger.Info(fmt.Sprintf("loaded %d pods", len(podList.Items)))
for _, pod := range podList.Items {
if pod.Spec.NodeName == "" {
diff --git a/perf-tools/scenarios/e2e_perf.go b/perf-tools/scenarios/e2e_perf.go
index 34595dd..664bf45 100644
--- a/perf-tools/scenarios/e2e_perf.go
+++ b/perf-tools/scenarios/e2e_perf.go
@@ -66,6 +66,7 @@ func (ts *E2EPerfScenario) Init(kubeClient *utils.KubeClient,
conf *framework.Co
return LoadScenarioConf(conf, ts.GetName(), ts.scenarioConf)
}
+//nolint:funlen
func (eps *E2EPerfScenario) Run(results *utils.Results) {
scenarioResults := results.CreateScenarioResults(eps.GetName())
maxWaitTime := time.Duration(eps.commonConf.MaxWaitSeconds) *
time.Second
diff --git a/perf-tools/scenarios/node_fairness.go
b/perf-tools/scenarios/node_fairness.go
index b7f7417..1ae2651 100644
--- a/perf-tools/scenarios/node_fairness.go
+++ b/perf-tools/scenarios/node_fairness.go
@@ -66,6 +66,7 @@ func (nfs *NodeFairnessScenario) Init(kubeClient
*utils.KubeClient, conf *framew
return LoadScenarioConf(conf, nfs.GetName(), nfs.scenarioConf)
}
+//nolint:funlen
func (nfs *NodeFairnessScenario) Run(results *utils.Results) {
scenarioResults := results.CreateScenarioResults(nfs.GetName())
maxWaitTime := time.Duration(nfs.commonConf.MaxWaitSeconds) *
time.Second
diff --git a/release-tools/merge_pr.sh b/release-tools/merge_pr.sh
index 7ea0e24..541f505 100755
--- a/release-tools/merge_pr.sh
+++ b/release-tools/merge_pr.sh
@@ -66,7 +66,7 @@ function leave() {
}
# prompt for a continue response
-function continue() {
+function continueFn() {
PS3=$1
echo "$2"
select _ in "yes" "no"; do
@@ -81,7 +81,7 @@ function continue() {
# check the jira reference
function check_jira() {
- if ! grep -q '^\[YUNIKORN-[0-9]\+]' <<< $(echo $1)
+ if ! grep -q '^\[YUNIKORN-[0-9]\+]' <<< "$1"
then
echo "Subject does not contain a jira reference."
echo "The subject line of the commit must follow the pattern:"
@@ -93,7 +93,7 @@ function check_jira() {
echo "$1"
echo "---"
echo "Please fix the subject during the commit, press any key to continue"
- read -n 1
+ read -r -n 1
fi
}
@@ -177,12 +177,12 @@ fi
# merge the PR
if ! git merge --squash "${PRBRANCH}"
then
- if ! continue "manually fix merge conflicts? " "Merge failed, conflict must
be resolved before continuing"
+ if ! continueFn "manually fix merge conflicts? " "Merge failed, conflict
must be resolved before continuing"
then
echo "aborting"
abort
fi
- if ! continue "continue? " "Please fix any conflicts and 'git add'
conflicting files..."
+ if ! continueFn "continue? " "Please fix any conflicts and 'git add'
conflicting files..."
then
echo "aborting"
abort
@@ -228,7 +228,7 @@ if [ -n "${CONFLICT}" ]; then
fi
echo " committer:"$'\t'"${SIGNED}"
-if ! continue "Commit changes? " ""
+if ! continueFn "Commit changes? " ""
then
echo "aborting before commit"
abort
@@ -241,7 +241,7 @@ then
abort
fi
-if continue "push change to ${COMMITBRANCH}? " "Merge completed local ref:
${MERGEBRANCH}"
+if continueFn "push change to ${COMMITBRANCH}? " "Merge completed local ref:
${MERGEBRANCH}"
then
if ! git push "${REMOTE}" "${MERGEBRANCH}":"${COMMITBRANCH}"
then
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
