[2.2.x] Running octodns-sync twice in one job fails

[solvaholic]

ℹ️ This issue does not affect solvaholic/octodns-sync@v2.1.x ℹ️

Description

Running octodns-sync (this Action) multiple times in one job fails when scripts/install.sh tries to clone octodns/octodns again to an existing octodns-src directory.

Expected Behavior

Multiple runs of octodns-sync in one job should not block each other. For example, you may have a public.yaml and a private.yaml and want to test both in one job.

Actual Behavior

The first time I tried it, both runs seemed to succeed OK 🤷

The next time, the first step succeeded and the repeat step failed because Git refused to re-use the octodns-src directory.

Possible Fix

Teach scripts/install.sh to recognize and use an existing clone.

Steps to Reproduce

      # Run octodns to test current config
      - name: Test current configuration
        id: octodns-test
        uses: solvaholic/octodns-sync@issue50
        with:
          config_path: public.yaml
          doit: 'no'
          add_pr_comment: 'Yes'
          octodns_ref: ${{ steps.pick-ref.outputs.octodns_ref }}
          pr_comment_token: ${{ github.token }}
      # Test again in the same job, to see what happens
      - name: Test current configuration again
        id: octodns-test2
        uses: solvaholic/octodns-sync@issue50
        with:
          config_path: public.yaml
          doit: 'no'
          add_pr_comment: 'Yes'
          octodns_ref: ${{ steps.pick-ref.outputs.octodns_ref }}
          pr_comment_token: ${{ github.token }}

Context

Your Environment

• Version used: 7fa64b6
• Link to your project: n/a

[solvaholic]

The first time I tried it, both runs seemed to succeed OK 🤷

I see it! On the first run I had octodns_ref set to v0.9.12, which caused scripts/install.sh to use pip install rather than cloning the repo.

octodns-sync/scripts/install.sh

Lines 6 to 18 in 7fa64b6

	if [[ "$_ver" = v* ]]; then
	# Assume input 'version' is an available release
	curl -O "https://raw.githubusercontent.com/octodns/octodns/$_ver/requirements.txt"
	pip install "octodns==${_ver#v}" -r requirements.txt
	else
	# Assume input 'version' is a Git ref in octodns/octodns
	if git clone --single-branch --branch "$_ver" --no-tags \
	https://github.com/octodns/octodns.git octodns-src; then
	pip install ./octodns-src -r ./octodns-src/requirements.txt
	else
	exit 1
	fi
	fi

For the next test I made octodns_ref a branch name, master, which caused scripts/install.sh to try to re-clone the repo in the second step.

A proper solution should account for this scenario as well:

      # Run octodns to test the first time, with a branch name
      - name: Test current configuration
        id: octodns-test
        uses: solvaholic/octodns-sync@7fa64b6
        with:
          octodns_ref: master
      # Test again in the same job, with a version tag
      - name: Test current configuration again
        id: octodns-test2
        uses: solvaholic/octodns-sync@7fa64b6
        with:
          octodns_ref: v0.9.12
      # Test again in the same job, with a different branch name
      - name: Test current configuration again again
        id: octodns-test3
        uses: solvaholic/octodns-sync@7fa64b6
        with:
          octodns_ref: super-feature
      # Test again in the same job, with a different version tag
      - name: Test current configuration again again again
        id: octodns-test4
        uses: solvaholic/octodns-sync@7fa64b6
        with:
          octodns_ref: v0.9.11

It may suffice to rm -rf ./octodns-src at the beginning of scripts/install.sh. I think it seems safer, at least in the meantime, to exit 1 if command -v octodns-sync evaluates true. (And output a helpful warning, of course.)

I'd like that (exit 1 if octodns-sync command is already present) as a solution, IF this Action also documents a method to, with one workflow job, run octodns-sync on more than one config .yaml.

[solvaholic]

I set up a workflow for testing and demonstrated this error:

I'll check command -v octodns-sync early in scripts/install.sh and ask it to say something more descriptive about why this won't work.

[solvaholic]

a3a3885 and subsequent commits handle this issue (octodns-sync is already installed) early and validate the octodns_ref provided. Now the git clone failure is prevented/avoided, and if command -v octdns-sync then the error looks like this:

[solvaholic]

While the current solution is, I guess, a solution to the problem, it's not the solution I want.

What I want is for solvaholic/octodns-sync to give you a way to run multiple times in one job, with different or same configurations.

Why, though? Like, apart from the fact I think it should work I'm not thinking of the use case. If you have one, I'd like to hear about it. Please re-open this issue or open a new one and let's discuss it.

In the meantime I added the wontfix label to draw attention to the fact this issue isn't really solved.

---

In case the changes I made today broke something for you, please let me know about that too. If need be we can revert them to:

octodns-sync/scripts/install.sh

Lines 1 to 18 in 7fa64b6

	#!/bin/bash
	# Install octodns and dependencies
	_ver="$OCTODNS_REF"
	if [[ "$_ver" = v* ]]; then
	# Assume input 'version' is an available release
	curl -O "https://raw.githubusercontent.com/octodns/octodns/$_ver/requirements.txt"
	pip install "octodns==${_ver#v}" -r requirements.txt
	else
	# Assume input 'version' is a Git ref in octodns/octodns
	if git clone --single-branch --branch "$_ver" --no-tags \
	https://github.com/octodns/octodns.git octodns-src; then
	pip install ./octodns-src -r ./octodns-src/requirements.txt
	else
	exit 1
	fi
	fi

[travislikestocode]

Hi there. Thanks for the work on this awesome action! I'd like to run the action 2x in the same job for the use case you described: deploying public and private DNS which have different config files. Yes it can be done in another job but in my opinion it's not worth the overhead of spinning up a new container.

What if install.sh outputs an environment variable or file to indicate that it's been run already and then have a check at the beginning which simply quits (with exit code 0 so it doesn't halt the workflow) if it's not the first run?

[solvaholic]

Hi @travislikestocode 👋 You're welcome! And thank you for bringing this up 🙇

What if install.sh outputs an environment variable or file to indicate that it's been run already and then have a check at the beginning which simply quits (with exit code 0 so it doesn't halt the workflow) if it's not the first run?

I dig it. That way if octodns-sync is already available the action skips the install and moves on.

In my imagination there were workflow job step configuration details to reconcile, on re-run. For example, what if in step 1 octodns_ref is v0.9.12 and in step 2 it's, idk, main?

Looking at the action's inputs now, though, I think octodns_ref is the only one that can create such a conflict. We could ignore it if octodns-sync is already installed, and log about that.

I'd like to run the action 2x in the same job for the use case you described: deploying public and private DNS which have different config files.

Would it meet your needs to run the action 2x, with effectively the same octodns_ref each time?

[travislikestocode]

@solvaholic ah good catch. It's fine for me not to handle the case where a different ref is used since I'm using the same version for both sync runs. I did this for my own fork to get things working. I'll submit a PR. For the record it is also necessary to delete the plan file from an earlier run, on subsequent runs.

[solvaholic]

For the record it is also necessary to delete the plan file from an earlier run, on subsequent runs.

Yikes! I think it'll help for the Action to pass the plan output as an output.

On subsequent runs, it's ok to just delete the previous plan and log files, right?

Unless we find a reason to preserve (and rotate?) them, I'd be ok to just rm -f the path near the beginning of run.sh and echo a message about it.

octodns-sync/scripts/run.sh

Lines 8 to 9 in 92eadc9

	_logfile="${GITHUB_WORKSPACE}/octodns-sync.log"
	_planfile="${GITHUB_WORKSPACE}/octodns-sync.plan"

[travislikestocode]

On subsequent runs, it's ok to just delete the previous plan and log files, right?

Works for me!

Unless we find a reason to preserve (and rotate?) them, I'd be ok to just rm -f the path near the beginning of run.sh and echo a message about it.

Sounds good. I did that but also added the output you suggested so we should be good

Created #66 for this

[solvaholic]

Thank you @travislikestocode 🙇

I'll check out this and #67 and follow up quick as I can, it may be a few days tho (family and work schedules are dicey just now). At the latest, I'll follow up on Mon 05 Jul.

[travislikestocode]

My pleasure!

I'll check out this and #67 and follow up quick as I can, it may be a few days tho (family and work schedules are dicey just now). At the latest, I'll follow up on Mon 05 Jul.

Sweet, no rush! Take your time :)

[solvaholic]

#66 has been merged, which fixes this in main. Thank you @travislikestocode ! 🙇