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

fix(declarative) initialize hash for empty config #8425

Merged
merged 2 commits into from
Feb 18, 2022
Merged

Conversation

locao
Copy link
Contributor

@locao locao commented Feb 16, 2022

Summary

This change adds back the configuration_hash entry to the /status endpoint when using dbless mode/node is a dataplane and no configuration has been loaded.

See #8214 for reference.

Full changelog

  • configuration_hash returns "00000000000000000000000000000000" when no configuration has been loaded.
  • Tests updated.

FT-2358

@locao locao requested a review from shaneutt February 16, 2022 20:38
@locao locao requested review from dndx and hbagdi February 16, 2022 20:39
@locao locao added this to the 2.8.0 milestone Feb 16, 2022
@hbagdi
Copy link
Member

hbagdi commented Feb 18, 2022

LGTM
@fffonion Could you please take a look at this as well?

Copy link
Contributor

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

From the KIC perspective all we're looking for is a data-point which indicates that the gateway has not been initialized with a configuration yet in DBLESS mode (primary reason being to trigger configuration re-push if the gateway container crashes). Having all 0's as an indicator of empty instead of null is workable for us 👍

@@ -187,6 +187,7 @@ local constants = {
DECLARATIVE_PAGE_KEY = "declarative:page",
DECLARATIVE_LOAD_KEY = "declarative_config:loaded",
DECLARATIVE_HASH_KEY = "declarative_config:hash",
DECLARATIVE_EMPTY_CONFIG_HASH = string.rep("0", 32),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Good to see this moving to a constant rather than just being a magic string.

@flrgh flrgh requested a review from fffonion February 18, 2022 16:45
@locao locao merged commit c8b103e into master Feb 18, 2022
@locao locao deleted the fix/empty_config_value branch February 18, 2022 19:22
kikito pushed a commit that referenced this pull request Feb 24, 2022
* fix(declarative) initialize hash for empty config

* docs(CHANGELOG) feature description
kikito pushed a commit that referenced this pull request Feb 24, 2022
* fix(declarative) initialize hash for empty config

* docs(CHANGELOG) feature description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants