[2/3] Add Inline Solution for ConfigMaps #593
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
linki
commented
Feb 23, 2024
linki
force-pushed
the
configmap-and-secret-refs
branch
4 times, most recently
from
16324de
to
f4ab763
Compare
linki
commented
Feb 27, 2024
linki
force-pushed
the
configmap-inline-another-try
branch
from
c463eef
to
4371dc9
Compare
linki
force-pushed
the
configmap-inline-another-try
branch
2 times, most recently
from
9c57062
to
f732160
Compare
linki
force-pushed
the
configmap-inline-another-try
branch
from
f732160
to
bb4468a
Compare
linki
force-pushed
the
configmap-inline-another-try
branch
from
bb4468a
to
ee45eb2
Compare
linki
changed the title
This was referenced Mar 7, 2024
linki
force-pushed
the
configmap-inline-another-try
branch
from
ee45eb2
to
d24e5a4
Compare
linki
commented
Mar 7, 2024
pkg/core/stack_resources.go
Outdated
|
|
||
| base := &v1.ConfigMap{ | ||
| ObjectMeta: metaObj, | ||
| Data: configResource.ConfigMap.Data, |
There was a problem hiding this comment.
I think for the (original) reference approach, we could just do a
client.CoreV1().ConfigMaps(metaObj.Namespace).Get(ctx, configResource.GetName(), metav1.GetOptions{})to fetch the reference ConfigMap's data and other fields. We'd need to pass-in the Kubernetes client for that but everything else would stay the same.
Still leaves the race condition problem...
linki
commented
Mar 7, 2024
| // ConfigMap removed | ||
| if desiredConfigMap == nil { | ||
| if existingConfigMap != nil { | ||
| err := c.client.CoreV1().ConfigMaps(existingConfigMap.Namespace).Delete(ctx, existingConfigMap.Name, metav1.DeleteOptions{}) |
There was a problem hiding this comment.
This doesn't handle the case where a ConfigMap with the same name already exists in the cluster but isn't defined in the StackSet definition as a configurationResource. In that case the resource would be deleted. I think the same would happen for HPAs, Ingress, RouteGroups etc. but maybe we want to do an owner check similar to how we already do it for reference ConfigMaps (where we don't touch the resource if it's not owned by us).
linki
force-pushed
the
configmap-inline-another-try
branch
from
d24e5a4
to
df18af0
Compare
linki
changed the title
linki
changed the title
linki
force-pushed
the
configmap-inline-another-try
branch
from
df18af0
to
b6abb34
Compare
linki
force-pushed
the
configmap-inline-another-try
branch
from
b6abb34
to
19a2f7a
Compare
linki
force-pushed
the
configmap-inline-another-try
branch
2 times, most recently
from
433bc16
to
ef7d88d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a solution for Inline ConfigMaps. It follows the design of other inline resources such as HPAs. It could be used as a blueprint for inline PCS as well.
Please also see the previous and follow up PRs that are based on this one:
Let's also do this: