add: WPB-25752 add multi-ingress verification script - #6
Conversation
…5 version for wire-server and nginx-ingress-services
mohitrajain
changed the title
| print_wire_snippet() { | ||
| local d="$1" | ||
| cat <<EOF | ||
|
|
There was a problem hiding this comment.
this UI of this is a little misleading, I was checking all the printed fields even though some of them were actually passing, I think the prints you do, for example:
MISSING/WRONG: add cargohold.config.aws.multiIngress."nginz-https.kiwi.bochuoi.com": https://assets.kiwi.bochuoi.com
are sufficient and actually a better feedback for the user
also, something is wrong with your logic here (or maybe its intended) but you only get a printout of this for the last failing domain (if multiple had failed, only print_wire_snippet() for the last one will be shown)
lets either drop this completely or print it out with generic values and have the user fill in the blanks with MISSING/WRONG prints from, im leaning towards removal
| && ok "nginz additional_external_env_domains contains $d" \ | ||
| || fail "add $d to nginz.nginx_conf.additional_external_env_domains" | ||
|
|
||
| yaml_has_list_value '.cannon.nginx_conf.additional_external_env_domains' "$d" "$WIRE_VALUES" \ |
There was a problem hiding this comment.
value is under .cannon.config.nginx_conf.additional_external_env_domains
| && ok "cannon additional_external_env_domains contains $d" \ | ||
| || fail "add $d to cannon.nginx_conf.additional_external_env_domains" | ||
|
|
||
| if yq -e 'galley.config.settings.conversationCodeURI? != null' "$WIRE_VALUES" >/dev/null 2>&1; then |
There was a problem hiding this comment.
missing trailing .
also, I don't think this check belongs on the additional domains check, since its not domain specific, rather global
you do a similar check for cargoholds s3DownloadEndpoint in the main ingress checks and print the same message, perhaps this is better fitted over there?
| || fail "set config.dns.base: $NGINX_DOMAIN" | ||
|
|
||
| [[ "$(yaml_get '.config.dns.https' "$NGINX_VALUES")" == "nginz-https.${NGINX_DOMAIN}" ]] \ | ||
| && ok "config.dns.https" \ |
There was a problem hiding this comment.
you are missing a || fail clause here
2 participants
add multi-ingress verification script verified for 5.25 version for wire-server and nginx-ingress-services
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Issues
Briefly describe the issue you have solved or implemented with this pull request. If the PR contains multiple issues, use a bullet list.
Causes (Optional)
Briefly describe the causes behind the issues. This could be helpful to understand the adopted solutions behind some nasty bugs or complex issues.
Solutions
Briefly describe the solutions you have implemented for the issues explained above.
Dependencies (Optional)
If there are some other pull requests related to this one (e.g. new releases of frameworks), specify them here.
Needs releases with:
Testing
Test Coverage (Optional)
How to Test
Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.
Notes (Optional)
Specify here any other facts that you think are important for this issue.
Attachments (Optional)
Attachments like images, videos, etc. (drag and drop in the text box)
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.