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

ignore TTL & update config if app version doesn't match config version #612

Merged
merged 2 commits into from
Jul 28, 2022

Conversation

joeshaw
Copy link
Member

@joeshaw joeshaw commented Jul 28, 2022

Fixes an issue where the local config did not have the Go starter kit
after updating until at least 5 minutes afterward.

The issue went something like this:

  1. You run fastly update to get the latest software
  2. The config gets updated to the latest version, but unsupported features (like the new Go support) are dropped when the config gets reserialized to disk
  3. The software gets updated
  4. The config file gets updated again (from Ensure application configuration is updated after fastly update #610), but it's still the old software version so there's no change
  5. The config file has its last checked time updated to now
  6. You run the new software and its check as to whether to update the config file sees that it's been updated within the TTL (5m) and doesn't update it
  7. The Go starter kit is sill missing

This PR fixes the issue by having the runtime check for whether the config needs to be updated ignore the TTL when the version in the config file does not match the currently-running CLI version. That way, the first time you run the CLI after updating, it will always update the config file first.

The change from #610 should probably be reverted? It doesn't seem to have any effect since the same update happens immediately before downloading the software. I wasn't 100% sure on that and didn't include it in this PR but I can add it if it's the right thing to do.

Fixes an issue where the local config did not have the Go starter kit
after updating until at least 5 minutes afterward.
@joeshaw
Copy link
Member Author

joeshaw commented Jul 28, 2022

Looks like a spurious CI failure but I don't have the perms to rerun the jobs.

Copy link
Member

@fgsch fgsch left a comment

Choose a reason for hiding this comment

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

Thank you!

I think we should revert #610 but it's your call. We can always do it in a separate PR.

This reverts commit 7844e6e.

This PR was trying to address the problem described in fastly#612 but because
it's still running in the old version, it didn't get the additional new
config values.
Copy link
Member

@fgsch fgsch left a comment

Choose a reason for hiding this comment

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

Re-approving 👍

@fgsch fgsch merged commit c3e322e into fastly:main Jul 28, 2022
@joeshaw joeshaw deleted the joeshaw/update-config-on-version-change branch July 28, 2022 15:59
@Integralist Integralist added the bug Something isn't working label Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants