Need to enhance validation of custom repositories

[Sergii Golovatiuk]

I modified URL for repository on UI and made a mistake in Repositories. I added

"deb http://10.108.0.2:8080/2014.2.2-6.1/ubuntu/x86_64 mos6.1/ main restricted"

which is not correct as it contains "mos6.1/" as a release instead of "mos6.1" which is not correct.
There were no verification if this release accessible
Deployment failed with very weird description "Failed to execute hook 'shell". As UX experience it's very hard to understand what happened, requires to check the actual problem from astute container.

Environment:
[root@nailgun mnt]# curl -Ssl http://10.108.0.2:8000/api/version/ | python -mjson.tool
{
    "api": "1.0",
    "astute_sha": "0bd72c72369e743376864e8e8dabfe873d40450a",
    "auth_required": true,
    "build_id": "2015-05-24_22-08-23",
    "build_number": "464",
    "feature_groups": [
        "mirantis"
    ],
    "fuel-library_sha": "889c2534ceadf8afd5d1540c1cadbd913c0c8c14",
    "fuel-ostf_sha": "9a5f55602c260d6c840c8333d8f32ec8cfa65c1f",
    "fuelmain_sha": "5c8ebddf64ea93000af2de3ccdb4aa8bb766ce93",
    "nailgun_sha": "76441596e4fe6420cc7819427662fa244e150177",
    "openstack_version": "2014.2.2-6.1",
    "production": "docker",
    "python-fuelclient_sha": "e19f1b65792f84c4a18b5a9473f85ef3ba172fce",
    "release": "6.1",
    "release_versions": {
        "2014.2.2-6.1": {
            "VERSION": {
                "api": "1.0",
                "astute_sha": "0bd72c72369e743376864e8e8dabfe873d40450a",
                "build_id": "2015-05-24_22-08-23",
                "build_number": "464",
                "feature_groups": [
                    "mirantis"
                ],
                "fuel-library_sha": "889c2534ceadf8afd5d1540c1cadbd913c0c8c14",
                "fuel-ostf_sha": "9a5f55602c260d6c840c8333d8f32ec8cfa65c1f",
                "fuelmain_sha": "5c8ebddf64ea93000af2de3ccdb4aa8bb766ce93",
                "nailgun_sha": "76441596e4fe6420cc7819427662fa244e150177",
                "openstack_version": "2014.2.2-6.1",
                "production": "docker",
                "python-fuelclient_sha": "e19f1b65792f84c4a18b5a9473f85ef3ba172fce",
                "release": "6.1"
            }
        }
    }
}

[Vladimir Sharshov (vsharshov)]

Bad UX, but do not affect deployment. Moving to 8.0

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/238845

[Artem Roma (aroma-x)]

Though we have a pre-deployment check [1] there still may be situations when errors are made in other parts of the description and that would only effect on the stage of installing of packages which is not under control of Nailgun code. The bug is best illustration here. Sure thing we must enhance the check: test that repository is viable too. But, in my opinion, it is not so simply to achieve as some design of such code and the way it is integrated in current logic of processing repository data must be done.

[1] https://github.com/openstack/fuel-web/blob/master/nailgun/nailgun/task/task.py#l1445-l1449

[OpenStack Infra (hudson-openstack)]

Change abandoned by Artem Roma (<email address hidden>) on branch: master
Review: https://review.openstack.org/238845
Reason: Please, see comments to the linked bug

[Ihor Kalnytskyi (ikalnytskyi)]

Artem, pre-deployment check ensures that repo host is reachable. It doesn't check that repo is in *correct* format, and it shouldn't be done so late. So it makes sense to have your patch in master or I missed something.

[Artem Roma (aroma-x)]

Igor, to be honest I don't think the patch is silver bullet here. Down to final version the code only checks that suite name doesn't contain backward slashes in it in case when 'casual' way of repository description is used. It is so particular case for amount of code being introduced. Continuing, if we add possibility to deploy CentOS nodes, in that case user may also be able to supply custom rpm repos. Those also should be validated which means - sticking to logic of my changes - adding more small specific checks. IMO that looks like simply not right approach. Besides, as I've mentioned in my previous comment, user also might make mistakes in other parts of description - sections names, type of repository (considering apt format). What I have in mind is separate check that will perform some "real life" interaction with particular repository (e.g. `apt-get update` operation). Hence test will be more general and effective, and we can drop all intermediate validation. This pretty rough description of the idea though, so I would gladly hear other opinions about the matter.

[Alexander Kislitsky (akislitsky)]

We passed SCF in 8.0. Moving the bug to 9.0.