solvaholic / octodns-sync

GitHub Action to test and deploy DNS settings with OctoDNS
MIT License
28 stars 14 forks source link

Action successful even though task fails #23

Closed barnumbirr closed 3 years ago

barnumbirr commented 3 years ago

What I did

Run octodns-sync action to update DNS records of domain.

What I expected to happen

Github Action fails as task failed

What happened instead

Task Publish fails with the following stacktrace:

2020-12-05T03:15:43  [139881296222016] INFO  OvhProvider[ovh] apply: making changes
2020-12-05T03:15:43  [139881296222016] INFO  OvhProvider[ovh] _apply: zone=martinsimon.me., len(changes)=6
Traceback (most recent call last):
  File "/usr/local/bin/octodns-sync", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.9/site-packages/octodns/cmds/sync.py", line 38, in main
    manager.sync(eligible_zones=args.zone, eligible_targets=args.target,
  File "/usr/local/lib/python3.9/site-packages/octodns/manager.py", line 344, in sync
    total_changes += target.apply(plan)
  File "/usr/local/lib/python3.9/site-packages/octodns/provider/base.py", line 95, in apply
    self._apply(plan)
  File "/usr/local/lib/python3.9/site-packages/octodns/provider/ovh.py", line 101, in _apply
    getattr(self, '_apply_{}'.format(class_name).lower())(zone_name,
  File "/usr/local/lib/python3.9/site-packages/octodns/provider/ovh.py", line 114, in _apply_update
    self._apply_delete(zone_name, change)
  File "/usr/local/lib/python3.9/site-packages/octodns/provider/ovh.py", line 121, in _apply_delete
    if self._is_valid_dkim(existing.values[0]):
  File "/usr/local/lib/python3.9/site-packages/octodns/provider/ovh.py", line 361, in _is_valid_dkim
    is_valid_key = self._is_valid_dkim_key(value)
  File "/usr/local/lib/python3.9/site-packages/octodns/provider/ovh.py", line 374, in _is_valid_dkim_key
    base64.decodestring(bytearray(key, 'utf-8'))
AttributeError: module 'base64' has no attribute 'decodestring'

Yet the job run still is marked as successful.

Additional info

octodns-sync should probably catch errors and fail jobs when tasks fail.

solvaholic commented 3 years ago

Hi @barnumbirr 👋

Thank you so much for reporting this! Please forgive my delays in following up with you.

octodns-sync should probably catch errors and fail jobs when tasks fail.

I agree. I think piping to tee here is what drops the non-zero exit code:

https://github.com/solvaholic/octodns-sync/blob/7d33ae676691d23a657749ddf30bea12e4c28f23/entrypoint.sh#L39-L47

/bin/sh doesn't know about pipefail so I think either use /bin/bash or do something other than | tee to write the output to a file. I tried switching to /bin/bash in #24 and noticed an error (#25) I'm not sure is OK. I'll dig into it more this week, or this coming weekend.

solvaholic commented 3 years ago

@barnumbirr Thanks again for reporting this. If the changes from #24 don't correct the issue, please do re-open this. 🙇