| Message ID | 20220211084433.10614-1-pavel@zhukoff.net |
|---|---|
| State | New |
| Headers | show |
| Series | sanity.class: Add possibility to configure networking check | expand |
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
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
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
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
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
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"
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(-)