-
Notifications
You must be signed in to change notification settings - Fork 185
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
ENT-11970: Moved masterfiles-stage checks for git in sync so that they can work properly #5577
Closed
Closed
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,9 +112,6 @@ git_deploy_refspec() { | |
mkdir -p "${temp_stage}/.git" | ||
cp "${local_mirrored_repo}/HEAD" "${temp_stage}/.git/" | ||
|
||
if git_check_is_in_sync "${local_mirrored_repo}" "${temp_stage}" "$2"; then | ||
return 0 | ||
fi | ||
|
||
########################## 3. SET PERMISSIONS ON POLICY SET | ||
chown -R root:root "${temp_stage}" || error_exit "Unable to chown '${temp_stage}'" | ||
|
@@ -238,9 +235,6 @@ git_cfbs_deploy_refspec() { | |
mkdir -p "${temp_stage}/out/masterfiles/.git" | ||
cp "${local_mirrored_repo}/HEAD" "${temp_stage}/out/masterfiles/.git/" | ||
|
||
if git_check_is_in_sync "${local_mirrored_repo}" "${temp_stage}/out/masterfiles" "$2"; then | ||
return 0 | ||
fi | ||
|
||
########################## 3. SET PERMISSIONS ON POLICY SET | ||
chown -R root:root "${temp_stage}" || error_exit "Unable to chown '${temp_stage}'" | ||
|
@@ -385,15 +379,22 @@ git_masterstage() { | |
# Depends on $GIT_URL, $ROOT, $MASTERDIR, $GIT_REFSPEC | ||
check_git_installed | ||
git_setup_local_mirrored_repo "$( dirname "$ROOT" )" | ||
if [ "x$1" = "xtrue" ]; then | ||
if git_check_is_in_sync "$( dirname "$ROOT" )" "$MASTERDIR" "$GIT_REFSPEC"; then | ||
return 1 # in sync => nothing to do | ||
git_check_is_in_sync "$local_mirrored_repo" "$MASTERDIR" "$GIT_REFSPEC" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be nice to mention in a comment here that we are abusing the previous function setting a global variable so we know local_mirrored_repo |
||
local in_sync=$? | ||
if [ "$1" = "true" ]; then | ||
if [ "$in_sync" = "0" ]; then | ||
return 1 # in sync => nothing to do | ||
else | ||
return 0 # not in sync => update available | ||
fi | ||
else | ||
if [ "$in_sync" != "0" ]; then | ||
git_deploy_refspec "$MASTERDIR" "${GIT_REFSPEC}" | ||
echo "Successfully deployed '${GIT_REFSPEC}' from '${GIT_URL}' to '${MASTERDIR}' on $(date)" | ||
else | ||
return 0 # not in sync => update available | ||
echo "'${GIT_REFSPEC}' from '${GIT_URL}' has no changes. Will not update." | ||
fi | ||
fi | ||
git_deploy_refspec "$MASTERDIR" "${GIT_REFSPEC}" | ||
echo "Successfully deployed '${GIT_REFSPEC}' from '${GIT_URL}' to '${MASTERDIR}' on $(date)" | ||
} | ||
|
||
git_cfbs_masterstage() { | ||
|
@@ -402,15 +403,22 @@ git_cfbs_masterstage() { | |
check_git_installed | ||
check_cfbs_installed | ||
git_setup_local_mirrored_repo "$( dirname "$ROOT" )" | ||
if [ "x$1" = "xtrue" ]; then | ||
if git_check_is_in_sync "$( dirname "$ROOT" )" "$MASTERDIR" "$GIT_REFSPEC"; then | ||
return 1 # in sync => nothing to do | ||
git_check_is_in_sync "$local_mirrored_repo" "$MASTERDIR" "$GIT_REFSPEC" | ||
local in_sync=$? | ||
if [ "$1" = "true" ]; then | ||
if [ "$in_sync" = "0" ]; then | ||
return 1 # in sync => nothing to do | ||
else | ||
return 0 # not in sync => update available | ||
fi | ||
else | ||
if [ "$in_sync" != "0" ]; then | ||
git_cfbs_deploy_refspec "$MASTERDIR" "${GIT_REFSPEC}" | ||
echo "Successfully built and deployed '${GIT_REFSPEC}' from '${GIT_URL}' to '${MASTERDIR}' on $(date) with cfbs" | ||
else | ||
return 0 # not in sync => update available | ||
echo "'${GIT_REFSPEC}' from '${GIT_URL}' has no changes. Will not update." | ||
fi | ||
fi | ||
git_cfbs_deploy_refspec "$MASTERDIR" "${GIT_REFSPEC}" | ||
echo "Successfully built and deployed '${GIT_REFSPEC}' from '${GIT_URL}' to '${MASTERDIR}' on $(date) with cfbs" | ||
} | ||
|
||
svn_branch() { | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should this not work? The mirror repo is already cloned/fetched/checked out, right? It's here on purpose to ensure early return without overwriting all of
/var/cfengine/masterfiles
with the same revision.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem here I think is that we have copied HEAD into temp_stage so it is always the same.
As-is I never got updates. There may be another cause but if you check my notes in ENT-11970 you might see some details that shed some light on the situation.
I imagine it worked for you while testing so not sure what is going on. Maybe grab a nightly build and try it out there. That's what I was doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I didn't test this very much. I was planning to test the whole set of changing how and where from masterfiles-stage.sh is run, but then I got busy with other things before finishing the set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see it now! But IMHO the error here is to compare with
${temp_stage}
, we should compare with$MASTERDIR
(or whatever it's called) and keep doing it here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, looks like that's enough: #5579 (and https://github.com/cfengine/nova/pull/2284)