sanity.class: Add possibility to configure networking check

Message ID 20220211084433.10614-1-pavel@zhukoff.net
State New
Headers show
Series sanity.class: Add possibility to configure networking check | expand

Commit Message

Pavel Zhukov Feb. 11, 2022, 8:44 a.m. UTC
Previously sanity checker required all uri listed in
CONNECTIVITY_CHECK_URIS to be available for networking check. It caused
selftests failures due to temprorary example.com unavailability.
Add CONNECTIVITY_CHECK_ALL variable to change this behaviour and check
for any host from the list to be available it allows to specify
"failover" uri and/or speed checker up if few uris have been specified.

Default value is set to "1" for backward compatibility.

Signed-off-by: Pavel Zhukov <pavel.zhukov@huawei.com>
---
 meta/classes/sanity.bbclass                   | 44 ++++++++++++-------
 .../distro/include/default-distrovars.inc     |  5 ++-
 2 files changed, 32 insertions(+), 17 deletions(-)

Comments

Ross Burton Feb. 11, 2022, 11:18 a.m. UTC | #1
On Fri, 11 Feb 2022 at 08:44, Pavel Zhukov <pavel@zhukoff.net> wrote:
> +        for uri in test_uris:
> +            try:
> +                if check_all:
> +                    fetcher = bb.fetch2.Fetch(test_uris, data)
> +                else:
> +                    fetcher = bb.fetch2.Fetch([uri], data)
> +                fetcher.checkstatus()
> +                return retval

This seems like it's overcomplicating things to me.  example.com was
having uptime issues, and we can change it to www.yoctoproject.org so
that we know in advance of expected downtime, and have direct contact
with the people responsible for keeping it up. So let's just change
the URL to www.yoctoproject.org instead of adding more variables and
non-trivial logic.

Ross
Jacob Kroon Feb. 11, 2022, 11:23 a.m. UTC | #2
On 2/11/22 12:18, Ross Burton wrote:
> On Fri, 11 Feb 2022 at 08:44, Pavel Zhukov <pavel@zhukoff.net> wrote:
>> +        for uri in test_uris:
>> +            try:
>> +                if check_all:
>> +                    fetcher = bb.fetch2.Fetch(test_uris, data)
>> +                else:
>> +                    fetcher = bb.fetch2.Fetch([uri], data)
>> +                fetcher.checkstatus()
>> +                return retval
> 
> This seems like it's overcomplicating things to me.  example.com was
> having uptime issues, and we can change it to www.yoctoproject.org so
> that we know in advance of expected downtime, and have direct contact
> with the people responsible for keeping it up. So let's just change
> the URL to www.yoctoproject.org instead of adding more variables and
> non-trivial logic.
> 

I vote for removing the check altogether.

As I understand it, the check was added in order to prevent
hard-to-interpret error messages when network failed. I can live with that.

Jacob
Ross Burton Feb. 11, 2022, 11:39 a.m. UTC | #3
On Fri, 11 Feb 2022 at 11:23, Jacob Kroon <jacob.kroon@gmail.com> wrote:
> I vote for removing the check altogether.
>
> As I understand it, the check was added in order to prevent
> hard-to-interpret error messages when network failed. I can live with that.

I think the check should remain, and example.com has served well for
some time now (since 2016 in
423a2645377bff2cd7292e1dc9796eeb3595e937).

We moved to example.com to speed up the check: the old tests were for
two yoctoproject.org URLs and it downloads the entire page, so that
took time.

If we can get a minimal page added to yoctoproject.org that doesn't
involve hitting WordPress, then we can own the URL and have fast
responses.

Ross
Jacob Kroon Feb. 11, 2022, 12:08 p.m. UTC | #4
On 2/11/22 12:39, Ross Burton wrote:
> On Fri, 11 Feb 2022 at 11:23, Jacob Kroon <jacob.kroon@gmail.com> wrote:
>> I vote for removing the check altogether.
>>
>> As I understand it, the check was added in order to prevent
>> hard-to-interpret error messages when network failed. I can live with that.
> 
> I think the check should remain, and example.com has served well for
> some time now (since 2016 in
> 423a2645377bff2cd7292e1dc9796eeb3595e937).
> 
> We moved to example.com to speed up the check: the old tests were for
> two yoctoproject.org URLs and it downloads the entire page, so that
> took time.
> 
> If we can get a minimal page added to yoctoproject.org that doesn't
> involve hitting WordPress, then we can own the URL and have fast
> responses.
> 

I suppose if the intention is that the Yocto server will never ever be
down, the check is ok. But if it is ever going to need to go down
because of maintenance you will break bitbake builds for everyone in the
world, right ? Then I don't think users will be happy, even though the
downtime was planned and publicly announced on some website.

Jacob
Richard Purdie Feb. 11, 2022, 1:13 p.m. UTC | #5
On Fri, 2022-02-11 at 13:08 +0100, Jacob Kroon wrote:
> On 2/11/22 12:39, Ross Burton wrote:
> > On Fri, 11 Feb 2022 at 11:23, Jacob Kroon <jacob.kroon@gmail.com> wrote:
> > > I vote for removing the check altogether.
> > > 
> > > As I understand it, the check was added in order to prevent
> > > hard-to-interpret error messages when network failed. I can live with that.
> > 
> > I think the check should remain, and example.com has served well for
> > some time now (since 2016 in
> > 423a2645377bff2cd7292e1dc9796eeb3595e937).
> > 
> > We moved to example.com to speed up the check: the old tests were for
> > two yoctoproject.org URLs and it downloads the entire page, so that
> > took time.
> > 
> > If we can get a minimal page added to yoctoproject.org that doesn't
> > involve hitting WordPress, then we can own the URL and have fast
> > responses.
> > 
> 
> I suppose if the intention is that the Yocto server will never ever be
> down, the check is ok. But if it is ever going to need to go down
> because of maintenance you will break bitbake builds for everyone in the
> world, right ? Then I don't think users will be happy, even though the
> downtime was planned and publicly announced on some website.

The reason we added this check in the first place was that people had
interesting corporate environments and would blame the project when it didn't
work. The check made sure we highlighted where the issue really was and in that
sense it was really useful to new users. It was all about the new user
experience. Experienced users can just disable it or reconfigure it very easily.

I think using yoctoproject.org would be ok, but as mentioned, we need a plain
page for this, reducing unneeded overhead is good. Let's see if we can sort
something out. The yocto project site is cloud hosted these days and should be
reliable with good uptime.

Cheers,

Richard

Patch

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index f288b4c84c..695776bba3 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -326,6 +326,7 @@  def check_connectivity(d):
     # using the same syntax as for SRC_URI. If the variable is not set
     # the check is skipped
     test_uris = (d.getVar('CONNECTIVITY_CHECK_URIS') or "").split()
+    check_all = (d.getVar('CONNECTIVITY_CHECK_ALL') or "1") == "1"
     retval = ""
 
     bbn = d.getVar('BB_NO_NETWORK')
@@ -341,22 +342,33 @@  def check_connectivity(d):
         data = bb.data.createCopy(d)
         data.delVar('PREMIRRORS')
         data.delVar('MIRRORS')
-        try:
-            fetcher = bb.fetch2.Fetch(test_uris, data)
-            fetcher.checkstatus()
-        except Exception as err:
-            # Allow the message to be configured so that users can be
-            # pointed to a support mechanism.
-            msg = data.getVar('CONNECTIVITY_CHECK_MSG') or ""
-            if len(msg) == 0:
-                msg = "%s.\n" % err
-                msg += "    Please ensure your host's network is configured correctly.\n"
-                msg += "    If your ISP or network is blocking the above URL,\n"
-                msg += "    try with another domain name, for example by setting:\n"
-                msg += "    CONNECTIVITY_CHECK_URIS = \"https://www.yoctoproject.org/\""
-                msg += "    You could also set BB_NO_NETWORK = \"1\" to disable network\n"
-                msg += "    access if all required sources are on local disk.\n"
-            retval = msg
+        err = None
+        for uri in test_uris:
+            try:
+                if check_all:
+                    fetcher = bb.fetch2.Fetch(test_uris, data)
+                else:
+                    fetcher = bb.fetch2.Fetch([uri], data)
+                fetcher.checkstatus()
+                return retval
+            except Exception as e:
+                err = "{} \n {}".format(err, e)
+                if check_all:
+                    break
+
+
+        # Allow the message to be configured so that users can be
+        # pointed to a support mechanism.
+        msg = data.getVar('CONNECTIVITY_CHECK_MSG') or ""
+        if len(msg) == 0:
+            msg = "%s.\n" % err
+            msg += "    Please ensure your host's network is configured correctly.\n"
+            msg += "    If your ISP or network is blocking the above URL,\n"
+            msg += "    try with another domain name, for example by setting:\n"
+            msg += "    CONNECTIVITY_CHECK_URIS = \"https://www.yoctoproject.org/\""
+            msg += "    You could also set BB_NO_NETWORK = \"1\" to disable network\n"
+            msg += "    access if all required sources are on local disk.\n"
+        retval = msg
 
     return retval
 
diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
index fb0f1097da..867d694146 100644
--- a/meta/conf/distro/include/default-distrovars.inc
+++ b/meta/conf/distro/include/default-distrovars.inc
@@ -54,4 +54,7 @@  KERNEL_IMAGETYPES ??= "${KERNEL_IMAGETYPE}"
 # fetch from the network (and warn you if not). To disable the test set
 # the variable to be empty.
 # Git example url: git://git.yoctoproject.org/yocto-firewall-test;protocol=git;rev=master;branch=master
-CONNECTIVITY_CHECK_URIS ?= "https://www.example.com/"
+CONNECTIVITY_CHECK_URIS ?= "https://www.example.com/ https://www.yoctoproject.org"
+# Define is all CONNECTIVITY_CHECK_URIS are required to be available or it's enough to have some
+# of them reachable for networking check
+CONNECTIVITY_CHECK_ALL ?= "0"