Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Reduce DNSSEC refresh change log noise#6279

Merged
dneuman64 merged 1 commit intoapache:masterfrom
rawlinp:fix-dnssec-refresh-cl
Oct 18, 2021
Merged

Reduce DNSSEC refresh change log noise#6279
dneuman64 merged 1 commit intoapache:masterfrom
rawlinp:fix-dnssec-refresh-cl

Conversation

@rawlinp
Copy link
Copy Markdown
Contributor

@rawlinp rawlinp commented Oct 13, 2021

Only create a changelog entry if any keys actually changed or an error
occurred. Also, differentiate between errors storing into Traffic Vault
and other types of errors.

Closes: #6268


Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

Attempt to refresh the DNSSEC keys in quick succession. If there are no keys refreshed, there should be no change log entry created.

If this is a bugfix, which Traffic Control versions contained the bug?

6.0.0

PR submission checklist

@rawlinp rawlinp added Traffic Ops related to Traffic Ops improvement The functionality exists but it could be improved in some way. labels Oct 13, 2021
@rawlinp
Copy link
Copy Markdown
Contributor Author

rawlinp commented Oct 13, 2021

For some reason, the TP tests are failing to get past the initialize stage, and it seems to be an issue unrelated to this PR.

@zrhoffman
Copy link
Copy Markdown
Member

For some reason, the TP tests are failing to get past the initialize stage, and it seems to be an issue unrelated to this PR.

That is #6283, fixed in #6284

@rawlinp rawlinp force-pushed the fix-dnssec-refresh-cl branch from ec334ea to 3a43b9d Compare October 14, 2021 17:10
Copy link
Copy Markdown
Contributor

@srijeet0406 srijeet0406 left a comment

Choose a reason for hiding this comment

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

Hit the cdns/dnsseckeys/refresh api endpoint back to back and made sure that changelog msgs were not getting generated.
All tests pass, code looks good.

@srijeet0406
Copy link
Copy Markdown
Contributor

@rawlinp You might need to rebase for the GHA TP tests to pass.

Only create a changelog entry if any keys actually changed or an error
occurred. Also, differentiate between errors storing into Traffic Vault
and other types of errors.

Closes: apache#6268
@rawlinp rawlinp force-pushed the fix-dnssec-refresh-cl branch from 3a43b9d to 0025023 Compare October 14, 2021 23:21
@rawlinp
Copy link
Copy Markdown
Contributor Author

rawlinp commented Oct 14, 2021

Rebased!

@ocket8888
Copy link
Copy Markdown
Contributor

You're gonna need to do it again to get #6290 in your branch

@rawlinp
Copy link
Copy Markdown
Contributor Author

rawlinp commented Oct 15, 2021

@ocket8888 I did, but hit another spurious TP test failure (unrelated to this PR).

@dneuman64 dneuman64 merged commit 49b9a03 into apache:master Oct 18, 2021
@rawlinp rawlinp deleted the fix-dnssec-refresh-cl branch October 18, 2021 19:46
@rawlinp rawlinp added this to the 6.0.1 milestone Oct 20, 2021
zrhoffman pushed a commit that referenced this pull request Nov 5, 2021
Only create a changelog entry if any keys actually changed or an error
occurred. Also, differentiate between errors storing into Traffic Vault
and other types of errors.

Closes: #6268

(cherry picked from commit 49b9a03)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

improvement The functionality exists but it could be improved in some way. Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DNSSEC refresh change log entry unnecessary if no keys are refreshed

5 participants