Merge ~smoser/cloud-init:fix/1134036-locale-message-change into cloud-init:master

[Scott Moser]

Z99-cloud-locale-test.sh: shorten message and change locale to C.UTF-8.

Two basic changes here to the behavior of Z99-cloud-locale-test.sh
when a locale found in LC_* settings is not present on the system:

a.) shorten the message.
b.) change the locale to C.UTF-8.

LP: #1134036

This can be tested with:
$ rm -f ~/.cloud-locale-test.skip
$ LC_ALL=fr_FR.UTF-8 sh ./tools/Z99-cloud-locale-test.sh

The message shown at this point is:

| Your provided locale (LC_ALL=fr_FR.UTF-8) is not available and has been
| changed to C.UTF-8 (LC_ALL=C.UTF-8). To install native locales:
| sudo apt-get install locales language-pack-fr

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:4d94c6f9cb1a244997634399547137d5cd868d2c
https://jenkins.ubuntu.com/server/job/cloud-init-ci/92/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/92/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:8ad644088052baad0e3db31a632dba45b1aec605
https://jenkins.ubuntu.com/server/job/cloud-init-ci/93/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/93/rebuild

review: Approve (continuous-integration)

[Steve Langasek (vorlon)]

From https://bugs.launchpad.net/ubuntu/+source/maas/+bug/1134036/comments/20, this addresses the first point (no broken locale at login), but not the second or third points (no banner message for English locale requested; no banner message on minimal images). Are you disagreeing with those points, or is this just not implemented yet as part of this MP?

[Scott Moser (smoser)]

Steve,

Of the 4 points addressed, I think that I addressed
a.) set LC_ALL=C.UTF-8 - this is done

b.) a long message - i shortened the message dramatically.

c.) do not show in minimal images - this is not addressed.
I'm not sure how we would address this. I'd prefer that cloud-init not have specific "This is a Canonical authored Ubuntu minimal image" code, so I'd like some generic mechanism to determine that. I'd appreciate any suggestions on how to do that.

d.) limited value of english message.
I generally disagree here, and suggest that it is "good enough" and better than silently changing user's environment variables. As you said "the vast majority of actual Ubuntu Server users are comfortable navigating a CLI in English even if it is not their first language."

I really don't want to spend much time on this. I'm not particularly convinced that the arguments in the bug warrant a change in behavior for all users, and I would not personally want to justify this behavioral change in a SRU. That puts me in the annoying position of having to maintain patches to prior Ubuntu stable releases.

So... I'm perfectly fine if you want to take over from here. I had this code open and thought I'd try to improve the situation a bit.

[Steve Langasek (vorlon)]

Improving the situation "a bit" does not address my concerns. As long as users of client-side English locales have such a message taking up any real estate at login, I consider this unresolved. From our discussion and the git history I don't see any evidence that this was subject to any sort of high-level design review when it was added; I don't think it's appropriate to have a higher barrier for its removal.

If you are concerned about not hard-coding implementation details of Ubuntu minimal images into this script, then the simplest way to address is to remove the warning message unconditionally. This would be consistent with the long-term design goals for openssh.

review: Disapprove

[Steve Langasek (vorlon)]

technical review of the changes, inline:

review: Needs Fixing

[Scott Moser (smoser)]

> Improving the situation "a bit" does not address my concerns. As long as
> users of client-side English locales have such a message taking up any real
> estate at login, I consider this unresolved. From our discussion and the git
> history I don't see any evidence that this was subject to any sort of high-
> level design review when it was added; I don't think it's appropriate to have
> a higher barrier for its removal.

Why do you think this is a "higher barrier"? We don't make *any* changes
to code without consideration. That is just generally accepted practice.
Changes that are user-facing do have higher barrier, and they do in
the ubuntu Stable Release Process as well.

>
> If you are concerned about not hard-coding implementation details of Ubuntu
> minimal images into this script, then the simplest way to address is to remove
> the warning message unconditionally. This would be consistent with the long-
> term design goals for openssh.

You linked to an upstream ssh bug opened over 10 years ago, with the last
comment made over 5 years ago. What do you consider "long-term" ?
Do you have some reason to believe that all of a sudden someone will change
that? Would you expect that that user-facing change would then be SRU'd
in ubuntu ?

[Scott Moser (smoser)]

I'll mark this "work in progress." This is probably all the effort I'll put forth on a fix. Feel free to submit your own solution for review.

diff --git a/tools/Z99-cloud-locale-test.sh b/tools/Z99-cloud-locale-test.sh
index 4978d87..050ee76 100644
--- a/tools/Z99-cloud-locale-test.sh
+++ b/tools/Z99-cloud-locale-test.sh
@@ -6,9 +6,8 @@
 # (c) 2012, Canonical Group, Ltd.
 #
 # This file is part of cloud-init. See LICENSE file for license information.
- 
-# Purpose: Detect invalid locale settings and inform the user
-#  of how to fix them.
+
+# Purpose: Detect invalid locale, change to C-UTF-8. and inform the user.
 
 locale_warn() {
     local bad_names="" bad_lcs="" key="" val="" var="" vars="" bad_kv=""
@@ -33,6 +32,10 @@ locale_warn() {
                 vars="${vars} $key=$val";;
         esac
     done
+
+    if [ "${bad_names#* LC_ALL}" != "${bad_names}" ]; then
+        bad_names=LC_ALL
+    fi
     for bad in $bad_names; do
         for var in ${vars}; do
             [ "${bad}" = "${var%=*}" ] || continue
@@ -47,14 +50,7 @@ locale_warn() {
     bad_kv=${bad_kv# }
     [ -n "$bad_lcs" ] || return 0
 
-    printf "_____________________________________________________________________\n"
-    printf "WARNING! Your environment specifies an invalid locale.\n"
-    printf " The unknown environment variables are:\n   %s\n" "$bad_kv"
-    printf " This can affect your user experience significantly, including the\n"
-    printf " ability to manage packages. You may install the locales by running:\n\n"
-
-    local bad invalid="" to_gen="" sfile="/usr/share/i18n/SUPPORTED"
-    local pkgs=""
+    local bad="" pkgs="" invalid="" to_gen="" sfile="/usr/share/i18n/SUPPORTED"
     if [ -e "$sfile" ]; then
         for bad in ${bad_lcs}; do
             grep -q -i "${bad}" "$sfile" &&
@@ -62,7 +58,6 @@ locale_warn() {
                 invalid="${invalid} ${bad}"
         done
     else
-        printf "  sudo apt-get install locales\n"
         to_gen=$bad_lcs
     fi
     to_gen=${to_gen# }
@@ -73,28 +68,20 @@ locale_warn() {
     done
     pkgs=${pkgs# }
 
-    if [ -n "${pkgs}" ]; then
-        printf "   sudo apt-get install ${pkgs# }\n"
-        printf "     or\n"
-        printf "   sudo locale-gen ${to_gen# }\n"
-        printf "\n"
-    fi
-    for bad in ${invalid}; do
-        printf "WARNING: '${bad}' is an invalid locale\n"
-    done
-
-    printf "To see all available language packs, run:\n"
-    printf "   apt-cache search \"^language-pack-[a-z][a-z]$\"\n"
-    printf "To disable this message for all users, run:\n"
-    printf "   sudo touch /var/lib/cloud/instance/locale-check.skip\n"
-    printf "_____________________________________________________________________\n\n"
-
-    # only show the message once
-    : > ~/.cloud-locale-test.skip 2>/dev/null || :
+    echo "Your provided locale (${bad_kv}) is not available and has been"
+    echo "changed to C.UTF-8 (LC_ALL=C.UTF-8). To install native locales:"
+    echo "  sudo apt-get install locales${pkgs:+ ${pkgs}}"
+    return 1
 }
 
-[ -f ~/.cloud-locale-test.skip -o -f /var/lib/cloud/instance/locale-check.skip ] ||
-    locale 2>&1 | locale_warn
+if [ ! -f ~/.cloud-locale-test.skip ] &&
+   [ ! -f /var/lib/cloud/instance/locale-check.skip ]; then
+   if locale 2>&1 | locale_warn; then
+      LC_ALL=C.UTF-8
+      # only show the message once
+      : > ~/.cloud-locale-test.skip 2>/dev/null || :
+   fi
+fi
 
 unset locale_warn
 # vi: ts=4 expandtab