Skip to content

Comments

Убрал лишние pull, исправил форматирование#91

Open
ovcharenko-di wants to merge 1 commit intofirstBitMarksistskaya:feature/first-bitfrom
ovcharenko-di:fix/pull
Open

Убрал лишние pull, исправил форматирование#91
ovcharenko-di wants to merge 1 commit intofirstBitMarksistskaya:feature/first-bitfrom
ovcharenko-di:fix/pull

Conversation

@ovcharenko-di
Copy link

@ovcharenko-di ovcharenko-di commented Jan 28, 2026

@nixel2007 решил убрать флаги --pull, потому что это мешает сборке без Registry (как обсуждали).

Насколько я понимаю, что даже если сборка идет с использованием Registry, то свежий образ и так уже есть локально.

Summary by CodeRabbit

  • Style

    • Updated whitespace and indentation formatting in build scripts for consistency.
  • Chores

    • Modified Docker image pull behavior by removing the --pull flag from select build steps across multiple build scripts.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 28, 2026

📝 Walkthrough

Walkthrough

Docker build scripts for Jenkins agents have been modified across shell and batch files, primarily removing --pull flags from certain docker build invocations and adjusting whitespace/indentation formatting throughout. No functional control flow changes are introduced.

Changes

Cohort / File(s) Summary
Docker pull flag removal
build-base-swarm-jenkins-agent.bat, build-base-swarm-jenkins-agent.sh
Removed --pull option from docker build commands in these base agent scripts; other builds retain the flag. Accompanying whitespace re-indentation in bat file.
Whitespace and indentation reformatting
build-oscript-swarm-agent.bat, build-oscript-swarm-agent.sh
Tab-to-space indentation adjustments in docker build command arguments (--pull, --build-arg DOCKER_REGISTRY_URL=library, -f oscript/Dockerfile). No functional changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

Possibly related PRs

Poem

🐰 Docker flags fall like morning dew,
Build scripts dance in spaces new,
--pull removed, indents align,
Jenkins agents, sleek and fine! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main changes: removal of unnecessary --pull flags and fixing formatting, which aligns with the primary modifications across all four build script files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nixel2007
Copy link
Member

Насколько я понимаю, что даже если сборка идет с использованием Registry, то свежий образ и так уже есть локально.

Это зависит от того, какой используется base-image и сколько стейджей в докерфайле. может быть base image и передается из локального кэша, но какой-нибудь downloader или builder собирается поверх штатного debian-slim/ubuntu и в итоге остается уязвим. я бы на твоем месте еще раз аккуратно проверил все докерфайлы, где ты убрал pull и посмотрел, может ли это повлиять на актуальность образа

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@build-base-swarm-jenkins-agent.sh`:
- Around line 22-31: The docker build command references an undefined variable
no_cache_arg causing --no-cache to be ignored; fix by defining no_cache_arg from
the existing NO_CACHE env check (same logic used to set last_arg) or simply
replace $no_cache_arg with $last_arg so the conditional no-cache behavior is
applied; update the script where NO_CACHE is checked and ensure the variable
used in the docker build invocation (no_cache_arg or last_arg) is the one
actually set.

In `@build-oscript-swarm-agent.sh`:
- Around line 22-30: The script uses $no_cache_arg in the docker build command
but never defines it; add a definition (e.g., set no_cache_arg="--no-cache" when
NO_CACHE is truthy, otherwise empty) before the docker build invocation so the
NO_CACHE behavior actually takes effect—place the assignment near where last_arg
is defined and reference the same shell variable name no_cache_arg.
🧹 Nitpick comments (1)
build-base-swarm-jenkins-agent.bat (1)

23-33: Add --pull consistently or make it configurable.

Lines 23-32 and 40-47 lack --pull, while line 14 includes it. Most build scripts in the repo use --pull consistently. Either add it to these build commands for consistency with the first build in this file and pattern across the codebase, or make it configurable via an environment variable toggle (e.g., DOCKER_PULL) if offline builds need support.

Optional toggle for pull behavior
@@
 if %NO_CACHE%=="true" (SET last_arg="--no-cache .") else (SET last_arg=".")
+if "%DOCKER_PULL%"=="true" (SET pull_arg=--pull) else (SET pull_arg=)

@@
 docker build ^
+    %pull_arg% ^
     --build-arg ONEC_USERNAME=%ONEC_USERNAME% ^
@@
 docker build ^
+    %pull_arg% ^
     --build-arg ONEC_USERNAME=%ONEC_USERNAME% ^

Comment on lines 22 to 31
docker build \
--pull \
--pull \
$no_cache_arg \
--build-arg DOCKER_REGISTRY_URL=library \
--build-arg DOCKER_REGISTRY_URL=library \
--build-arg BASE_IMAGE=ubuntu \
--build-arg BASE_TAG=20.04 \
--build-arg ONESCRIPT_PACKAGES="yard" \
-t ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}oscript-downloader:latest \
-f oscript/Dockerfile \
-f oscript/Dockerfile \
$last_arg
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all references to no_cache_arg in build scripts
rg -n '\bno_cache_arg\b' --glob 'build-*.sh'

Repository: firstBitMarksistskaya/onec-docker

Length of output: 467


🏁 Script executed:

#!/bin/bash
# Read the build-base-swarm-jenkins-agent.sh file to see full context
cat -n build-base-swarm-jenkins-agent.sh | head -50

Repository: firstBitMarksistskaya/onec-docker

Length of output: 1887


🏁 Script executed:

#!/bin/bash
# Check other build scripts that reference no_cache_arg
# Let's look at build-server.sh to see if it defines no_cache_arg
cat -n build-server.sh | head -35

Repository: firstBitMarksistskaya/onec-docker

Length of output: 1277


🏁 Script executed:

#!/bin/bash
# Search for how no_cache_arg is defined in any of the scripts
rg -B 5 'no_cache_arg' build-*.sh | head -80

Repository: firstBitMarksistskaya/onec-docker

Length of output: 2145


🏁 Script executed:

#!/bin/bash
# Let's check if no_cache_arg is defined anywhere in the entire file
grep -n 'no_cache_arg=' build-base-swarm-jenkins-agent.sh

Repository: firstBitMarksistskaya/onec-docker

Length of output: 59


🏁 Script executed:

#!/bin/bash
# Also check all build scripts to see if any of them define no_cache_arg
rg 'no_cache_arg\s*=' build-*.sh

Repository: firstBitMarksistskaya/onec-docker

Length of output: 59


Define no_cache_arg or remove it.

Line 24 references $no_cache_arg but it's never defined in this file or any other build script, so the variable silently expands to empty and the --no-cache flag is ignored. The NO_CACHE environment variable is checked and $last_arg is conditionally set, but $no_cache_arg is used instead in the docker build command, creating a logic error.

🧩 Suggested fix
last_arg='.'
+no_cache_arg=''
if [ "${NO_CACHE}" = 'true' ] ; then
-    last_arg='--no-cache .'
+    no_cache_arg='--no-cache'
fi
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 24-24: no_cache_arg is referenced but not assigned.

(SC2154)

🤖 Prompt for AI Agents
In `@build-base-swarm-jenkins-agent.sh` around lines 22 - 31, The docker build
command references an undefined variable no_cache_arg causing --no-cache to be
ignored; fix by defining no_cache_arg from the existing NO_CACHE env check (same
logic used to set last_arg) or simply replace $no_cache_arg with $last_arg so
the conditional no-cache behavior is applied; update the script where NO_CACHE
is checked and ensure the variable used in the docker build invocation
(no_cache_arg or last_arg) is the one actually set.

Comment on lines 22 to 30
docker build \
--pull \
--pull \
$no_cache_arg \
--build-arg DOCKER_REGISTRY_URL=library \
--build-arg DOCKER_REGISTRY_URL=library \
--build-arg BASE_IMAGE=eclipse-temurin \
--build-arg BASE_TAG=17 \
-t ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}oscript-jdk:latest \
-f oscript/Dockerfile \
-f oscript/Dockerfile \
$last_arg
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for no_cache_arg definition in build scripts
rg -n --glob 'build-*.sh' '\bno_cache_arg\b' -B 2 -A 2

Repository: firstBitMarksistskaya/onec-docker

Length of output: 2312


🏁 Script executed:

#!/bin/bash
# Get the full build-oscript-swarm-agent.sh file to understand context
git ls-files | grep -E 'build-oscript-swarm-agent\.sh'

Repository: firstBitMarksistskaya/onec-docker

Length of output: 105


🏁 Script executed:

#!/bin/bash
# Read the build-oscript-swarm-agent.sh file to check for no_cache_arg definition
cat -n build-oscript-swarm-agent.sh | head -50

Repository: firstBitMarksistskaya/onec-docker

Length of output: 1668


Define no_cache_arg variable before use.

Lines 24 and 33 reference $no_cache_arg but the variable is never set. It silently expands to empty, defeating the intent of the NO_CACHE check. Currently only last_arg is defined (lines 17–20).

Suggested fix
 last_arg='.'
+no_cache_arg=''
 if [ "${NO_CACHE}" = 'true' ] ; then
-    last_arg='--no-cache .'
+    no_cache_arg='--no-cache'
 fi
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 24-24: no_cache_arg is referenced but not assigned.

(SC2154)

🤖 Prompt for AI Agents
In `@build-oscript-swarm-agent.sh` around lines 22 - 30, The script uses
$no_cache_arg in the docker build command but never defines it; add a definition
(e.g., set no_cache_arg="--no-cache" when NO_CACHE is truthy, otherwise empty)
before the docker build invocation so the NO_CACHE behavior actually takes
effect—place the assignment near where last_arg is defined and reference the
same shell variable name no_cache_arg.

%last_arg%

docker build ^
--pull ^
Copy link
Member

Choose a reason for hiding this comment

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

например, вот здесь мы останемся на протухшем FROM ubuntu:18.04 для основного слоя

Copy link
Author

@ovcharenko-di ovcharenko-di Jan 29, 2026

Choose a reason for hiding this comment

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

ок, но я не могу придумать простого способа обойти это, сохранив возможность собирать образы без Registry

вижу несколько вариантов:

  1. Описать в документации эту особенность (meh)
  2. Отказаться от сборки образов без Registry (meh^2)
  3. Добавить в скрипты функцию, которая будет перед сборкой парсить докерфайлы и делать pull образов принудительно

Copy link
Member

Choose a reason for hiding this comment

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

Screenshot_2026-01-29-09-02-57-347_com openai chatgpt

Интересно, правда так можно?

Copy link
Author

Choose a reason for hiding this comment

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

изучу, спасибо

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.

2 participants