Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers #3337

jmurret · 2023-12-08T20:26:11Z

Changes proposed in this PR

add additional readonly config maps for client and server extra config settings as a temporary landing spot for those changes
- system was currently writing to /consul/config and then copying and transforming to consul/extra-config
- issue with this is current approach is the extra values then get applied from the transformed files in consul/extra-config but also the original files from /consul/config that was being used as a temporary staging spot to dump the data before it got transformed and loaded into consul/extra-config
- duplicate config does not cause issues for many things, but for segments configuration it applies these against consul catalog and runs into errors that the segment is already and use.
- in new approach, consul/extra-config is still the ultimate spot that system will load values from and the temporary "staging" area for this config is /consul/tmp/extra-config
remove extra-from-values.json from existing server and client config maps
update client and server check sum annotations so they still factor in the extra-config as part of the checksum.
set consul/extra-config in a -config-dir flag in the consul agent command so that each component that wants to load data from this area, does not have to set up -config-file flags.

Todos:

make similar changes to clients
validate that segments work completely and not just the initial error is gone.
verify upgrading an existing cluster to this configuration
consider locality having its own volume, rather than 2 volumes with extra in them
create backports for 1.1, 1.2, and 1.3

How I've tested this PR

How I expect reviewers to test this PR

👀

Checklist

Tests added
CHANGELOG entry added

ndhanushkodi

Looks great, thanks John!

ndhanushkodi · 2023-12-12T18:21:23Z

acceptance/tests/segments/segments_test.go

+				"client.enabled":     "true",
+				"client.join[0]":     "${CONSUL_FULLNAME}-server-0.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303",
+				"client.join[1]":     "${CONSUL_FULLNAME}-server-1.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303",
+				"client.join[2]":     "${CONSUL_FULLNAME}-server-2.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303",


Is this because in consul-k8s 1.0+, even if you deploy client agents, they won't actually join the server cluster? So this isn't config that's specific to segments, but rather just that segments need client agents to work? Maybe we could add a comment for just why we need the client join configuration

this is needed in whichever consul-k8s versions we backport to (even < 1.0) because of this this block in client-daemonset.yaml that unless you set this, there is not a way to pass the port you configured for the segment to client. In this case, I am configuring the client to join the server replicas on port 8303 which alpha segment was configured on.

i can add a comment in the test.

are you asking if segments will work on agentless? If so, I am not familiar enough with agentless to know whether this can work.

(comment added in a083657)

charts/consul/templates/client-config-configmap.yaml

Todos: * make similar changes to clients * potentially upgrade test? * consider locality having its own volume, rather than 2 volumes with extra in them

…and off

…t config) on clients and servers (#3337) * wip: testing with server works when you add segments as extraValues. Todos: * make similar changes to clients * potentially upgrade test? * consider locality having its own volume, rather than 2 volumes with extra in them * move extra-config out of /consul/config so it does not get applied twice * add comments about use of additional config maps * remove temporary inclusion of values.yaml in root that was used for hand off * get rid of temporary config.file * add segments test * test using 3 servers in a single cluster * add changelog * fix linting issues. * add comment to test. remove extra lines from config map. * fix bats tests --------- Co-authored-by: Nitya Dhanushkodi <nitya@hashicorp.com>

…g for segment config) on clients and servers into release/1.3.x (#3374) Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers (#3337) * wip: testing with server works when you add segments as extraValues. Todos: * make similar changes to clients * potentially upgrade test? * consider locality having its own volume, rather than 2 volumes with extra in them * move extra-config out of /consul/config so it does not get applied twice * add comments about use of additional config maps * remove temporary inclusion of values.yaml in root that was used for hand off * get rid of temporary config.file * add segments test * test using 3 servers in a single cluster * add changelog * fix linting issues. * add comment to test. remove extra lines from config map. * fix bats tests --------- Co-authored-by: John Murret <john.murret@hashicorp.com> Co-authored-by: Nitya Dhanushkodi <nitya@hashicorp.com>

…g for segment config) on clients and servers into release/1.2.x (#3373) Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers (#3337) * wip: testing with server works when you add segments as extraValues. Todos: * make similar changes to clients * potentially upgrade test? * consider locality having its own volume, rather than 2 volumes with extra in them * move extra-config out of /consul/config so it does not get applied twice * add comments about use of additional config maps * remove temporary inclusion of values.yaml in root that was used for hand off * get rid of temporary config.file * add segments test * test using 3 servers in a single cluster * add changelog * fix linting issues. * add comment to test. remove extra lines from config map. * fix bats tests --------- Co-authored-by: John Murret <john.murret@hashicorp.com> Co-authored-by: Nitya Dhanushkodi <nitya@hashicorp.com>

…t config) on clients and servers (#3337)

…g for segment config) on clients and servers into release/1.1.x (#3372) * no-op commit due to failed cherry-picking * Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers (#3337) * wip: testing with server works when you add segments as extraValues. Todos: * make similar changes to clients * potentially upgrade test? * consider locality having its own volume, rather than 2 volumes with extra in them * move extra-config out of /consul/config so it does not get applied twice * add comments about use of additional config maps * remove temporary inclusion of values.yaml in root that was used for hand off * get rid of temporary config.file * add segments test * test using 3 servers in a single cluster * add changelog * fix linting issues. * add comment to test. remove extra lines from config map. * fix bats tests --------- Co-authored-by: Nitya Dhanushkodi <nitya@hashicorp.com> --------- Co-authored-by: temp <temp@hashicorp.com> Co-authored-by: John Murret <john.murret@hashicorp.com> Co-authored-by: Nitya Dhanushkodi <nitya@hashicorp.com>

…t config) on clients and servers (#3337) * wip: testing with server works when you add segments as extraValues. Todos: * make similar changes to clients * potentially upgrade test? * consider locality having its own volume, rather than 2 volumes with extra in them * move extra-config out of /consul/config so it does not get applied twice * add comments about use of additional config maps * remove temporary inclusion of values.yaml in root that was used for hand off * get rid of temporary config.file * add segments test * test using 3 servers in a single cluster * add changelog * fix linting issues. * add comment to test. remove extra lines from config map. * fix bats tests --------- Co-authored-by: Nitya Dhanushkodi <nitya@hashicorp.com>

jmurret force-pushed the nd/net-6786-extra-config branch 2 times, most recently from 1de666f to 5d8771c Compare December 9, 2023 00:07

jmurret changed the title ~~wip: testing with server works when you add segments as extraValues.~~ Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers Dec 9, 2023

jmurret force-pushed the nd/net-6786-extra-config branch from d1889bb to d459015 Compare December 11, 2023 23:19

jmurret added backport/1.1.x Backport to release/1.1.x branch backport/1.2.x This release branch is no longer active. backport/1.3.x labels Dec 11, 2023

jmurret marked this pull request as ready for review December 11, 2023 23:26

jmurret requested review from david-yu and ndhanushkodi December 11, 2023 23:30

ndhanushkodi approved these changes Dec 12, 2023

View reviewed changes

jmurret force-pushed the nd/net-6786-extra-config branch 2 times, most recently from 79a148c to 65f84a7 Compare December 13, 2023 19:02

jmurret enabled auto-merge (squash) December 13, 2023 19:15

ndhanushkodi and others added 11 commits December 13, 2023 14:06

wip: testing with server works when you add segments as extraValues.

dc6a8a9

Todos: * make similar changes to clients * potentially upgrade test? * consider locality having its own volume, rather than 2 volumes with extra in them

move extra-config out of /consul/config so it does not get applied twice

0780e1c

add comments about use of additional config maps

2b395fc

remove temporary inclusion of values.yaml in root that was used for h…

42bbbe9

…and off

get rid of temporary config.file

39490dd

add segments test

1844960

test using 3 servers in a single cluster

015e7dc

add changelog

67d9b7f

fix linting issues.

5aff03a

add comment to test. remove extra lines from config map.

c21e567

fix bats tests

46219bb

jmurret force-pushed the nd/net-6786-extra-config branch from 65f84a7 to 46219bb Compare December 13, 2023 21:06

jmurret merged commit 445fba7 into main Dec 13, 2023
26 of 48 checks passed

jmurret deleted the nd/net-6786-extra-config branch December 13, 2023 21:42

hc-github-team-consul-core mentioned this pull request Dec 13, 2023

Backport of Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers into release/1.3.x #3374

Merged

7 tasks

jmurret added a commit that referenced this pull request Dec 14, 2023

Prevent extra-config from being loaded twice (and erroring for segmen…

6091782

…t config) on clients and servers (#3337)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers #3337

Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers #3337

jmurret commented Dec 8, 2023 •

edited

Loading

ndhanushkodi left a comment

ndhanushkodi Dec 12, 2023

jmurret Dec 12, 2023 •

edited

Loading

jmurret Dec 12, 2023

jmurret Dec 12, 2023

jmurret Dec 12, 2023

Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers #3337

Prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers #3337

Conversation

jmurret commented Dec 8, 2023 • edited Loading

Changes proposed in this PR

Todos:

How I've tested this PR

How I expect reviewers to test this PR

Checklist

ndhanushkodi left a comment

Choose a reason for hiding this comment

ndhanushkodi Dec 12, 2023

Choose a reason for hiding this comment

jmurret Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

jmurret Dec 12, 2023

Choose a reason for hiding this comment

jmurret Dec 12, 2023

Choose a reason for hiding this comment

jmurret Dec 12, 2023

Choose a reason for hiding this comment

jmurret commented Dec 8, 2023 •

edited

Loading

jmurret Dec 12, 2023 •

edited

Loading