teuthology-suite: automate -t argument default value

[kshtsk]

This change is required to determine which teuthology
branch should be used for scheduling a run for the
given ceph branch.

In order to enable the functionality of this patch we need
submit and merge qa/.teuthology to ceph master
with following content:

# cherry-pick this for py3 ready branches
teuthology_branch: master

And add to /etc/teuthology.yaml on sepia server the line

# remove this line when all ceph branches get py3 compatible qa tasks
teuthology_branch: py2

Obviously, all users have to update their teuthology sandboxes.
As far as other branches

[sebastian-philipp]

Hm. This looks suspiciously similar to a requirements.txt file in ceph/qa to me.

Researching a bit more and it turns out that we already have an explicitly dependency to teuthology in the tox.ini.

I suspect a proper requirements.txt won't work, right?

[kshtsk]

Hm. This looks suspiciously similar to a requirements.txt file in ceph/qa to me.

I'm not following what you're referring to?

Researching a bit more and it turns out that we already have an explicitly dependency to teuthology in the tox.ini.

I suspect a proper requirements.txt won't work, right?

I have no idea what you're asking for, teuthology does not use requirements.txt at the moment, for py3 it uses requirements3.txt and for py2 it uses requirements2.txt

[idryomov]

FWIW I don't think this is worth it, both the override file and the tying of teuthology_branch to suite_branch or ceph_branch. Particularly the latter -- teuthology repo is full of stale branches with fairly generic names (wip-fixes and alike).

I think having to specify --teuthology-branch py2 is just fine, but if the consensus is that py2/py3 automation is necessary, then only py2 branch should be treated specially IMO. Instead of adding an override file, perhaps there is a py2-specific marker that can be grepped for in qa/tasks directory?

[kshtsk]

FWIW I don't think this is worth it, both the override file and the tying of teuthology_branch to suite_branch or ceph_branch. Particularly the latter -- teuthology repo is full of stale branches with fairly generic names (wip-fixes and alike).

Regarding suite_branch and ceph_branch functionality it was already in the code, I just added a description of this hidden logic. Do you request to delete it? @djgalloway @gregsfortytwo do we need to keep this?

I think having to specify --teuthology-branch py2 is just fine, but if the consensus is that py2/py3 automation is necessary, then only py2 branch should be treated specially IMO.

The point is that to avoid pointing to the right branch by a user, especially for users who do not have a clue which branch to use.

Instead of adding an override file, perhaps there is a py2-specific marker that can be grepped for in qa/tasks directory?

This is overkill, it much easier to add single file.

[idryomov]

FWIW I don't think this is worth it, both the override file and the tying of teuthology_branch to suite_branch or ceph_branch. Particularly the latter -- teuthology repo is full of stale branches with fairly generic names (wip-fixes and alike).

Regarding suite_branch and ceph_branch functionality it was already in the code, I just added a description of this hidden logic. Do you request to delete it? @djgalloway @gregsfortytwo do we need to keep this?

Was suite_branch already there? I thought it was just ceph_branch, which goes back to the early days of ceph, when all tasks lived in the teuthology repo and runs were scheduled with a shell script.

I'm not requesting that you delete it, but merely trying to discourage adding more tangling. It would be nice to look at whether ceph_branch logic is still needed, though -- there shouldn't be any good reason for it now. It's certainly not needed for any of the recent releases, as we are using the same py2 branch for them.

I think having to specify --teuthology-branch py2 is just fine, but if the consensus is that py2/py3 automation is necessary, then only py2 branch should be treated specially IMO.

The point is that to avoid pointing to the right branch by a user, especially for users who do not have a clue which branch to use.

Instead of adding an override file, perhaps there is a py2-specific marker that can be grepped for in qa/tasks directory?

This is overkill, it much easier to add single file.

Well, conceptually this file is an override for /etc/teuthology.yaml. I would argue that if you introduce an override mechanism, it should handle all options, not just one of them. And that is definitely an overkill for the problem at hand because all we need is for teuthology to detect that py2 tasks would be used and "switch" from py3 to py2. No additional surface area and no configuration files committed anywhere.

[kshtsk]

Was suite_branch already there? I thought it was just ceph_branch, which goes back to the early days of ceph, when all tasks lived in the teuthology repo and runs were scheduled with a shell script.

The ceph_branch what I saw has nothing todo with teuthology repo and uses the ceph repo.
If it is not the case, then I misinterpreted the usage of ceph_branch, because in other place the suite_branch is used for for checking out the code of tests from ceph repo, not teuthology. And following that logic I just complemented with logic reflecting ceph_branch and suite_branch substition of options --ceph and --suite-branch arguments.

I'm not requesting that you delete it, but merely trying to discourage adding more tangling. It would be nice to look at whether ceph_branch logic is still needed, though -- there shouldn't be any good reason for it now. It's certainly not needed for any of the recent releases, as we are using the same py2 branch for them.

I think having to specify --teuthology-branch py2 is just fine, but if the consensus is that py2/py3 automation is necessary, then only py2 branch should be treated specially IMO.

The point is that to avoid pointing to the right branch by a user, especially for users who do not have a clue which branch to use.

Instead of adding an override file, perhaps there is a py2-specific marker that can be grepped for in qa/tasks directory?

This is overkill, it much easier to add single file.

Well, conceptually this file is an override for /etc/teuthology.yaml. I would argue that if you introduce an override mechanism, it should handle all options, not just one of them.

It could look like an override for teuthology.yaml but it is not, it should not be, nevertheless, it has same yaml format, it only means to take only teuthology branch parameter.

And that is definitely an overkill for the problem at hand because all we need is for teuthology to detect that py2 tasks would be used and "switch" from py3 to py2. No additional surface area and no configuration files committed anywhere.

Could you elaborate, I'm not following, would it be better to you instead of yaml.safe_load('qa/.teuthology'), use misc.sh('grep py2 qa/.teuthology'). Or you want to try run python3 somepy3only.script? What if those files are missing in code base? The config file is easy to cherrypick. It would be greate to see a PR with your idea how it is the best to detect py2 tasks? ;-)

[idryomov]

Was suite_branch already there? I thought it was just ceph_branch, which goes back to the early days of ceph, when all tasks lived in the teuthology repo and runs were scheduled with a shell script.

The ceph_branch what I saw has nothing todo with teuthology repo and uses the ceph repo.
If it is not the case, then I misinterpreted the usage of ceph_branch, because in other place the suite_branch is used for for checking out the code of tests from ceph repo, not teuthology. And following that logic I just complemented with logic reflecting ceph_branch and suite_branch substition of options --ceph and --suite-branch arguments.

I'm referring to this piece of code, where in the absence of explicit --teuthology-branch foo on the command line (or in the environment) teuthology switches to a teuthology branch with same name as ceph_branch:

...
elif not teuthology_branch:
    # Decide what branch of teuthology to use
    if util.git_branch_exists('teuthology', self.args.ceph_branch):
        teuthology_branch = self.args.ceph_branch

This behaviour goes way back, when all tasks lived in the teuthology repo and one had to use the same branch of ceph and teuthology. Those days are gone and I think we should look into dropping this logic.

This PR adds similar behaviour for suite_branch. I don't see what use case is being addressed by this change and I think it should be dropped regardless of how you choose to proceed with py2/py3.

[kshtsk]

This PR adds similar behaviour for suite_branch. I don't see what use case is being addressed by this change and I think it should be dropped regardless of how you choose to proceed with py2/py3.

thanks for the extra explanation, I'm actual fine with dropping it, just don't want to do it by my own will...

[idryomov]

Instead of adding an override file, perhaps there is a py2-specific marker that can be grepped for in qa/tasks directory?

This is overkill, it much easier to add single file.

Well, conceptually this file is an override for /etc/teuthology.yaml. I would argue that if you introduce an override mechanism, it should handle all options, not just one of them.

It could look like an override for teuthology.yaml but it is not, it should not be, nevertheless, it has same yaml format, it only means to take only teuthology branch parameter.

And that is definitely an overkill for the problem at hand because all we need is for teuthology to detect that py2 tasks would be used and "switch" from py3 to py2. No additional surface area and no configuration files committed anywhere.

Could you elaborate, I'm not following, would it be better to you instead of yaml.safe_load('qa/.teuthology'), use misc.sh('grep py2 qa/.teuthology').

No.

Or you want to try run python3 somepy3only.script? What if those files are missing in code base? The config file is easy to cherrypick. It would be greate to see a PR with your idea how it is the best to detect py2 tasks? ;-)

Yes, I think it should be relatively easy to detect that py2 branch is be used without any new configuration files, whether by trying to import/parse as python3 or just a grep for some pattern in one of .py files under qa/tasks. If that file is not there, do nothing (i.e. use master).

[kshtsk]

Yes, I think it should be relatively easy to detect that py2 branch is be used without any new configuration files, whether by trying to import/parse as python3 or just a grep for some pattern in one of .py files under qa/tasks. If that file is not there, do nothing (i.e. use master).

I disagree and think this is incorrect approach an awkward... teuthology code should NOT be related on any client python code, you anyway need to track exact file, in many branches, you anyway have to made a change, you have much more file operation to make that grep. In fact, grepping is logically the same to loading of exact yaml file, but computationally more expensive.

Summarizing, I do not support this idea, it is not relatively easy (in comparison of just reading a single file with known structure), and since it is out of the scope for this PR, we can continue discussing it in mailing list, where you can advise code examples of how to verify if your whole tree is py2 or py3 only, as well as regex examples.

[djgalloway]

I don't have a dog in this fight as I don't use teuthology as a developer. Basically strictly a machine locking tool as far as I'm concerned.

[kshtsk]

I don't see input from @gregsfortytwo who actually requested the feature.
I'm actually discouraged so I'm ok to close it.

[badone]

@gregsfortytwo ping?

[badone]

@kshtsk I, for one, think we should persist with this because I am seeing confusion about this in day-to-day operations. Let's just see if we can get more eyes on this.

[jdurgin]

I think automating this is a good idea. More folks are getting confused by the various branches, and it's not difficult to avoid users needing to consider it at all. The mechanism proposed here seems reasonable to me.

[idryomov]

LGTM. Since the consensus is that automating this is a good idea and others have been slow to respond, I'm going to go ahead and merge.

[idryomov]

Hrm, why is master protected to the point that not everyone can merge? This didn't use to be the case and not a practice we follow in other repos...

[kshtsk]

Hrm, why is master protected to the point that not everyone can merge? This didn't use to be the case and not a practice we follow in other repos...

as far as I understand the history, only admins allowed, because not everyone wants take responsibility of breaking test infrastructure in sepia lab?

[jdurgin]

@idryomov added you to the list of folks who can merge. perhaps we should revisit that and allow more broad access again

[kshtsk]

Now we need:

1. Follow up PR to ceph/master with qa/.teuthology_branch file commit with the contents:

master

2. Ask @djgalloway to add teuthology_branch: py2 to /etc/teuthology.yaml

[djgalloway]

Step 2 is done.

[kshtsk]

ceph/ceph#35349

[kshtsk]

Step 2 is done.

The order is important, sorry, forgot to notice about that.

[djgalloway]

K I just commented it back out. No worries, I should have waited.

[djgalloway]

Uncommented now that ceph/ceph#35349 is merged.

[kshtsk]

nice, will see how it goes