Add more warning for service name replacement #541
Merged
Conversation
This file contains hidden or 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
edwardrf
commented
Jul 11, 2024
| ! service "bad-service": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) | ||
| ! service "bad-service": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors | ||
| ! service "env-in-config": environment variable "CONFIG1" overridden by config | ||
| ! service "env-in-config": environment variable "CONFIG1" with value "http://Mistral:8000" contains a service name needs to be normalized, but it will be repalced by an existing config. service names in a config will not be normalized. |
There was a problem hiding this comment.
Warning about overridden env var contains a service name replacement
edwardrf
commented
Jul 11, 2024
| ! service "ui": service names were replaced in environment variable "API_URL": "http://mock-mistral:8000" | ||
| ! service "use-bad-service": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) | ||
| ! service "use-bad-service": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors | ||
| ! service "use-bad-service": service names were NOT replaced in environment variable "DB_URL": "bad-service:5432", only service with port mode host will be replaced |
There was a problem hiding this comment.
Warning about service name NOT being replaced.
|
I rather we fix #396 |
lionello
reviewed
Jul 12, 2024
src/pkg/cli/compose/convert.go
Outdated
| @@ -130,7 +137,11 @@ func ConvertServices(ctx context.Context, c client.Client, serviceConfigs compos | |||
|
|
|||
| // Check if the environment variable is an existing config; if so, mark it as such | |||
| if _, ok := slices.BinarySearch(config.Names, key); ok { | |||
| term.Warnf("service %q: environment variable %q overridden by config", svccfg.Name, key) | |||
| if serviceNameRegex != nil && serviceNameRegex.MatchString(*value) { | |||
| term.Warnf("service %q: The environment variable %q, with the value %q, contains a service name that needs to be normalized. however, it will be replaced by an existing configuration. service names within a configuration will not be normalized.", svccfg.Name, key, *value) | |||
There was a problem hiding this comment.
fmt.Printf("service %q: environment variable %q needs service name fix-up, but is overridden by config, which will not be fixed up.", svccfg.Name, key)
lionello
reviewed
Jul 12, 2024
src/pkg/cli/compose/convert.go
Outdated
| @@ -130,7 +137,11 @@ func ConvertServices(ctx context.Context, c client.Client, serviceConfigs compos | |||
|
|
|||
| // Check if the environment variable is an existing config; if so, mark it as such | |||
| if _, ok := slices.BinarySearch(config.Names, key); ok { | |||
| term.Warnf("service %q: environment variable %q overridden by config", svccfg.Name, key) | |||
| if serviceNameRegex != nil && serviceNameRegex.MatchString(*value) { | |||
| term.Warnf("service %q: The environment variable %q, with the value %q, contains a service name that needs to be normalized. however, it will be replaced by an existing configuration. service names within a configuration will not be normalized.", svccfg.Name, key, *value) | |||
There was a problem hiding this comment.
fmt.Printf("service %q: environment variable %q needs service name fix-up, but is overridden by config, which will not be fixed up.", svccfg.Name, key)
src/pkg/cli/compose/convert.go
Outdated
| @@ -143,6 +154,8 @@ func ConvertServices(ctx context.Context, c client.Client, serviceConfigs compos | |||
| }) | |||
| if val != *value { | |||
| term.Warnf("service %q: service names were replaced in environment variable %q: %q", svccfg.Name, key, val) | |||
| } else if nonReplaceServiceNameRegex != nil && nonReplaceServiceNameRegex.MatchString(*value) { | |||
| term.Warnf("service %q: service names in the environment variable %q (%q) were not replaced. only services with port mode set to host will be replaced.", svccfg.Name, key, val) | |||
src/pkg/cli/compose/convert.go
Outdated
| @@ -143,6 +154,8 @@ func ConvertServices(ctx context.Context, c client.Client, serviceConfigs compos | |||
| }) | |||
| if val != *value { | |||
| term.Warnf("service %q: service names were replaced in environment variable %q: %q", svccfg.Name, key, val) | |||
edwardrf
force-pushed
the
edw-add-validation-warning
branch
from
4e64473 to
45a7467
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.
The following questions has been repeatedly pop up when a service name replacement is needed, which prevents a successful deployment: