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 dokku-daemon.yml #184

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Update dokku-daemon.yml #184

merged 1 commit into from
Apr 2, 2024

Conversation

robotski
Copy link
Contributor

GitHub's SSH key changed

https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/githubs-ssh-key-fingerprints

Should possibly add the Ed25519 key (and maybe remove the RSA one?) but those seem like more significant changes.

@ltalirz
Copy link
Member

ltalirz commented Mar 20, 2024

Thanks a lot @robotski

@robotski
Copy link
Contributor Author

@ltalirz I've done a bit of debugging on my end, and it seems like the CI failure is being caused by a bug in Dokku.

If I run dokku apps:create example-app followed by dokku git:sync example-app https://github.com/heroku/node-js-getting-started b10a4d7a20a6bbe49655769c526a2b424e0e5d0b --build, then the the git sha according to dokku git:report is b10a4d7. If I then run git:sync again without the --build, the sha becomes b10a4d7a and doesn't change again from any combination of the two git:sync commands.

I've opened up an issue in the main repo: dokku/dokku#6770

@ltalirz
Copy link
Member

ltalirz commented Mar 27, 2024

hi @robotski , thanks for looking into this!

Reading through your comment, it seems to me that the git sha reported is actually correct in all cases?

git has some logic to determine what fraction of the sha to display, i.e. it's possible that it decides after some operation that it now needs to display a longer fraction of the sha in order for the displayed fraction to be unique.

if some code relies on these "short versions" of shas to be identical, then probably that is the bug (one should compare full shas instead)

mentioning @josegonzalez for info

@robotski
Copy link
Contributor Author

I wonder if it might be appropriate to add a --git-sha-long option to dokku git:report then? It doesn't make sense for this Ansible role to do manual Git operations, IMO.

The currently existing --git-sha option being called by module_utils/dokku_git.py (used by library/dokku_clone.py to figure out if the app has changed) effectively runs git "$APP_ROOT" rev-parse --short HEAD. The --short option by itself returns the shortest unique sha, but using --short=10 or --short=12 would prevent the inconsistency AND hash collisions for any repo smaller than the Linux kernel.

Or the sha comparison in dokku_clone.py could truncate the longer hash, but that seems like an incorrect solution to me.

@ltalirz
Copy link
Member

ltalirz commented Mar 27, 2024

I wonder if it might be appropriate to add a --git-sha-long option to dokku git:report then? It doesn't make sense for this Ansible role to do manual Git operations, IMO.

Yes. Happy to review a PR

@josegonzalez
Copy link
Member

The dokku side should probably just always use a long commit-sha. Don't know if I feel this is a breaking change or not though.

@robotski
Copy link
Contributor Author

robotski commented Apr 1, 2024

@ltalirz Could you please re-run CI when you get a chance? There's a new release of Dokku out that fixes the sha issue. Thanks!

@ltalirz
Copy link
Member

ltalirz commented Apr 2, 2024

In one instance the test on ubuntu2204 failed with

  TASK [dokku_bot.ansible_dokku : clone dokku-daemon] ****************************
  fatal: [instance]: FAILED! => {"changed": false, "cmd": "/usr/bin/git ls-remote origin -h refs/heads/0.0.2", "msg": "fatal: unable to access 'https://github.com/dokku/dokku-daemon.git/': GnuTLS recv error (-54): Error in the pull function.", "rc": 128, "stderr": "fatal: unable to access 'https://github.com/dokku/dokku-daemon.git/': GnuTLS recv error (-54): Error in the pull function.\n", "stderr_lines": ["fatal: unable to access 'https://github.com/dokku/dokku-daemon.git/': GnuTLS recv error (-54): Error in the pull function."], "stdout": "", "stdout_lines": []}

Not sure where this comes from? In any case I'll merge for now

@ltalirz ltalirz merged commit 1b47f2f into dokku:master Apr 2, 2024
6 of 7 checks passed
@ltalirz
Copy link
Member

ltalirz commented Apr 2, 2024

Apparently this has to do with http post buffer sizes

@robotski
Copy link
Contributor Author

Would it be possible to publish to Galaxy soon? The current release is unable to clone repos.

@robotski robotski deleted the patch-1 branch April 10, 2024 18:10
@ltalirz
Copy link
Member

ltalirz commented Apr 10, 2024

just pushed the tag, let me know whether it works (will take 30 minutes to publish to galaxy)

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.

3 participants