Skip to content

Cleanup jenkins scripts #7648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Cleanup jenkins scripts #7648

wants to merge 3 commits into from

Conversation

torcolvin
Copy link
Collaborator

The motivation to do this was that SG_TEST_BUCKET_POOL is explicitly set by this script and I think we should move to having these default values determined by the code and jenkins could optionally overwrite them.

This is a low priority change to review.

This change is no-op, there are no functional changes to the script except not requiring setting SG_TEST_BUCKET_POOL_SIZE and
SG_TEST_BUCKET_POOL_DEBUG before you call this script. Note, these are always currently set by the jenkins pipelines anyway, so the values would pass through. So this is also not a functional change with the way the jenkins jobs are set up.

  • Added shellcheck action because I use this locally and it helps when refactoring.
    • Excluded all non test scripts from checking, but it will
  • Fixed the scripts to make shellcheck happy
    • quoting
    • switch comparison to consistent for bash, if [ "${SG_TEST_X509:-}" == "true" -a "${COUCHBASE_SERVER_PROTOCOL}" != "couchbases" ]; then to use && instead of -a
    • switch GO_TEST_FLAGS to use a bash array so that it could be correctly quoted to appease shellcheck
  • add REQUIRED_VARS check in the case I have to run the script locally, so it runs faster
  • Play with -x / +x so that it doesn't output all the test [ ] and is easy to read. I used the output to debug things but I think there's enough information to debug. This might just add noise.
  • Remove GO_MODULES and XATTRS since GO_MODULES isn't relevant since 3.1 and XATTRS is no longer relevant as of 4.0. This script now is always sourced from the repo, so I'm not worried about changes.
  • Remove unused DOCKER_CBS_ROOT_DIR since /workspace is the only volume used for cbcollects.
  • Remove \t from stdbuf -oL grep -a -E '(--- (FAIL|PASS|SKIP):|github.com/couchbase/sync_gateway(/.+)?\t|TEST: |panic: )' since grep always complainted about it, and it didn't do what it said. The scripts still find the values we need for the ok or failed for finishing package.

Testing

  • ran ./jenkins-integration-build.sh -m locally to simulate MasterIntegration job.
  • use ./start_server.sh locally (I already use this to create testing)

Integration Tests

@Copilot Copilot AI review requested due to automatic review settings July 25, 2025 12:24
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR cleans up and modernizes Jenkins shell scripts by improving their reliability, maintainability, and tooling. The changes focus on removing deprecated variables, improving shell script best practices, and adding linting support.

Key changes:

  • Removed deprecated variables (GO_MODULES, XATTRS, DOCKER_CBS_ROOT_DIR) and simplified configuration
  • Enhanced shell script quality with shellcheck compliance (proper quoting, array usage, conditional operators)
  • Added GitHub Actions workflow for automated shellcheck linting

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
jenkins-integration-build.sh Major cleanup removing deprecated vars, adding validation, improving shell compliance
integration-test/start_server.sh Removed unused DOCKER_CBS_ROOT_DIR variable and improved error handling
integration-test/service-test.sh Fixed shebang from #/bin/sh to #!/bin/bash
integration-test/docker-compose.yml Removed unused volume mount for DOCKER_CBS_ROOT_DIR
.github/workflows/shell.yml Added new shellcheck workflow for automated shell script linting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant