Skip to content
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

Update nightly branch after successful build #3195

Merged
merged 23 commits into from
Jan 3, 2024
Merged

Conversation

zaneselvans
Copy link
Member

@zaneselvans zaneselvans commented Dec 28, 2023

Overview

This is part of #3140. I created a new PR into dev because this no longer affects the workflow file (and so doesn't need to go into main directly) and because there was a bunch of unnecessarily messy history in #3183.

  • Update the nightly branch if the scheduled nightly build is successful.
  • Use [[ modern bash conditionals ]] throughout the script.
  • Track success of various different steps in separate environment variables.
  • Use the && to combine exit statuses of the commands inside functions so we can track the success/failure of the entire chain of commands.
  • Send output from all of the functions to the logfile in the same way.
  • Skip the nightly build entirely if there haven't been any changes since the last successful build.

Testing

Kinda need to run a real scheduled deployment...

To-do list

@zaneselvans zaneselvans added the nightly-builds Anything having to do with nightly builds or continuous deployment. label Dec 28, 2023
@zaneselvans zaneselvans self-assigned this Dec 28, 2023
@zaneselvans
Copy link
Member Author

This seems to be working, but it's unable to actually do the FF merge to update the nightly tag for reasons that I don't understand.

I'm currently attempting to run the build-deploy-pudl action after:

  • setting nightly to point at dev
  • rebasing the update-nightly-branch branch off of dev

So nightly/dev is definitely in the history of this branch, and it should be possible to merge --ff-only. HOWEVER, for some reason the commit that's associated with the testing-2023-12-29 tag inside the container when the action runs is DIFFERENT from the commit that's associated with that tag on GitHub and in my local repo after the action runs:

In my local repo / on GitHub:

commit 898837b683f4fb312848683bd7e903c4608cedad (HEAD -> update-nightly-branch, tag: testing-2023-12-29, origin/update-nightly-branch)
Author: Zane Selvans <[email protected]>
Date:   Thu Dec 28 09:17:02 2023 -0600

    Don't fetch NIGHTLY_TAG

But if I check out the hash that's associated with the testing-2023-12-29 tag inside the container:

commit d47cacb9a6d75a544c4d737a0078e2bd6ff4ced7 (HEAD)
Author: Zane Selvans <[email protected]>
Date:   Thu Dec 28 09:17:02 2023 -0600

    Don't fetch NIGHTLY_TAG

It seems like it is caching a version of the repo from before I did the rebase.

Could this be the stale container problem? Where it's not using the most recent version of the container to run the build? But shouldn't it be updating the local repository inside the container when it runs?

function update_nightly_branch() {
    git config --unset http.https://github.com/.extraheader && \
    git config user.email "[email protected]" && \
    git config user.name "pudlbot" && \
    git remote set-url origin "https://pudlbot:$PUDL_BOT_PAT@github.com/catalyst-cooperative/pudl.git" && \
    git checkout "$NIGHTLY_TAG" && \
    git tag && \
    echo "Updating nightly branch to point at $NIGHTLY_TAG." && \
    git fetch origin nightly:nightly && \
    git checkout nightly && \
    git merge --ff-only "$NIGHTLY_TAG" && \
    git push -u origin
}

I guess maybe it's not. It's just checking out whatever $NIGHTLY_TAG is present in the repo in the container already and if the container is stale, then the commit associated with the tag will be stale.

@zaneselvans
Copy link
Member Author

I updated the nightly build script to delete the local (possibly stale) $NIGHTLY_TAG and re-fetch it from origin and now the hash for the container, local, and GitHub versions of the tag are all identical, and it is still unable to merge --ff-only so I don't understand what the problem is.

@jdangerx
Copy link
Member

jdangerx commented Dec 29, 2023 via email

@jdangerx
Copy link
Member

jdangerx commented Dec 29, 2023 via email

@jdangerx
Copy link
Member

@zaneselvans what's the logging output you see when it's trying to merge --ff-only and failing?

@zaneselvans
Copy link
Member Author

@jdangerx Maybe this doesn’t make sense but I wonder if there’s any way that the test-docker-build which builds the image and doesn’t push it to DockerHub and does cache the image locally on GitHub is interfering with the GCE deployment somehow. Like if we kick off build-deploy-pudl before test-docker-build has finished then there’s a conflict?

WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load

Copy link
Member Author

@zaneselvans zaneselvans left a comment

Choose a reason for hiding this comment

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

@jdangerx I think this is ready to merge? But a line-by-line look would be helpful to make sure all of the testing hacks have been removed.

.github/workflows/build-deploy-pudl.yml Show resolved Hide resolved
.github/workflows/build-deploy-pudl.yml Outdated Show resolved Hide resolved
.github/workflows/build-deploy-pudl.yml Show resolved Hide resolved
.github/workflows/build-deploy-pudl.yml Outdated Show resolved Hide resolved
docker/gcp_pudl_etl.sh Show resolved Hide resolved
docker/gcp_pudl_etl.sh Show resolved Hide resolved
docker/gcp_pudl_etl.sh Show resolved Hide resolved
docker/gcp_pudl_etl.sh Outdated Show resolved Hide resolved
docker/gcp_pudl_etl.sh Show resolved Hide resolved
docker/gcp_pudl_etl.sh Show resolved Hide resolved
Copy link
Member

@jdangerx jdangerx left a comment

Choose a reason for hiding this comment

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

This mostly looks great! Added a couple suggestions around the git stuff & a question about the sandbox/prod argument, and then I think we're good to go!

Also, the nicer the bash script gets, the easier it will be to move it all into Python with little incident :)

.github/workflows/build-deploy-pudl.yml Show resolved Hide resolved
.github/workflows/build-deploy-pudl.yml Show resolved Hide resolved
.github/workflows/build-deploy-pudl.yml Outdated Show resolved Hide resolved
docker/gcp_pudl_etl.sh Show resolved Hide resolved
docker/gcp_pudl_etl.sh Outdated Show resolved Hide resolved
docker/gcp_pudl_etl.sh Show resolved Hide resolved
docker/gcp_pudl_etl.sh Outdated Show resolved Hide resolved
docker/gcp_pudl_etl.sh Show resolved Hide resolved
docker/gcp_pudl_etl.sh Show resolved Hide resolved
Copy link
Member

@jdangerx jdangerx left a comment

Choose a reason for hiding this comment

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

Sweet, I hope it all still works 😬

.github/workflows/build-deploy-pudl.yml Show resolved Hide resolved
@zaneselvans zaneselvans marked this pull request as ready for review January 3, 2024 16:30
@zaneselvans zaneselvans merged commit 5a81260 into dev Jan 3, 2024
14 of 18 checks passed
@zaneselvans zaneselvans deleted the update-nightly-branch branch January 3, 2024 16:33
@zaneselvans
Copy link
Member Author

I will be amazed if it all still works on the first try. 😆

I went ahead and merged it all the way into main so that the nightly build will try and use the new workflow, and so (hopefully) the --ff-only merge will work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nightly-builds Anything having to do with nightly builds or continuous deployment.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants