akondrahman / IaCTesting

Placeholder for the research study related to IaC testing anti-patterns
3 stars 225 forks source link

Final Analysis #30

Closed akondrahman closed 3 years ago

akondrahman commented 3 years ago

@Talismanic

Thanks for all the hard work so far. As the final analysis step, you will need to categorize the bugs that appear in test scripts. For that

  1. separate commits for which bug-related keyword appears and is used to modify a test script
  2. apply categorization in a similar manner you did for your LANGETI paper
  3. Quantify how frequent are each categories
  4. Report names, definitions, examples, and frequency for each category.

I would like to see this done in a week or so, as the ICST deadline is approaching

For questions please let me know.

Talismanic commented 3 years ago
  1. and is used to modify a test script

Bhaiya, does this mean I will pick both of the following category?

  1. Bugs in only test script
  2. Bugs in both test script and dev script
akondrahman commented 3 years ago

Yes, @Talismanic both.

Talismanic commented 3 years ago

@akondrahman Bhai, I have done my labeling in the below Spreadsheet.

https://docs.google.com/spreadsheets/d/1aQWknk0pGBcfRX5JJ8h6UT5PbzJM7kJ-vua5QlYrr-E/edit?usp=sharing

akondrahman commented 3 years ago

@Talismanic

From your labeling I do not understand what are the bug types that appear in Ansible test scripts. Can you please mention the bug types that appear in Ansible test files in this thread?

Talismanic commented 3 years ago

@akondrahman Bhai,

SAL = skip ansible lint LOT = local only test AR = assertion roulette ED = External Dependency NEC = No Environment Cleanup

akondrahman commented 3 years ago

@Talismanic another question that I have is the spreadsheet ... does it contain data only from the Openstack dataset?

akondrahman commented 3 years ago

@Talismanic you need to identify bug categories from commit messages that map to a bug. The above-mentioned categories are test anti-patterns/test smells, not test bug categories.

Talismanic commented 3 years ago

@Talismanic you need to identify bug categories from commit messages that map to a bug. The above-mentioned categories are test anti-patterns/test smells, not test bug categories.

@akondrahman Bhai, I think I could not understand the task. Can you kindly give me one example? That will help a lot.

Talismanic commented 3 years ago

@akondrahman Bhai, I think I am missing the concept of bug-category here.

For example,

Here are couple of commit messages:

No 1

Add unit tests run command Update import statements and references skip library files in flake8 Updated excludes in flake8 flake8 fixes to rundb

No 2

Clean deprecation warning for version compare error  DEPRECATION WARNING  Using tests as filters is deprecated. Instead of using  result version compare  instead use  result is version compare . This feature will be removed in version 2.9.

No 3

Gate Bootstrap Host with updated CPU Map The different cloud providers for OpenStack CI sometimes make use of hardware which libvirt does not appropriately map. This results in gate check failures as the nova scheduler is unable to find a suitable host on which to build the test VM s. This patch implements an extra set of tasks into the bootstrap process which adjusts the libvirt cpu map to resolve this issue. Implements blueprint gate split Change Id I666f53e01066bf8bff4d28fa012eadae7c958116

What can be the probable categories here?

akondrahman commented 3 years ago

@Talismanic

Here is my categorization:

No 1: dependency bug, as an import needed to be fixed No 2: deprecation bug, as deprecation is explicitly mentioned No 3: cpu mapping ... shows the importance of considering hardware while testing IaC scripts

Let me know what you think.

akondrahman commented 3 years ago

@Talismanic also, can you clarify what datasets are using? Openstack or GitHub?

Talismanic commented 3 years ago

@Talismanic also, can you clarify what datasets are using? Openstack or GitHub?

Bhaiya, This was all Github Repos. But as I have the dataset and scripts, I can try to run the similar analysis on Openstack within a short time.

Talismanic commented 3 years ago

Let me know what you think.

Got the idea Bhaiya. I was checking whether any other research have done something. Found one who has classified the commit messages on a very high level.

https://cibse2020.ppgia.pucpr.br/images/artigos/4/S04_P1.pdf

I am giving my categorization. But I will try to not exceed 5-7 categories.

Talismanic commented 3 years ago

@akondrahman Bhai, I have done initial labeling on the unique commits. Unfortunately I used 15 categories. Link here.

Bug Category Definition:

Capacity Bug

Bug introduced due to shortage of infra capacity (cpu, memory or storage)

Cofiguration Bug

Bug introduced due to some mis-configuration or default value of configuration

Dependency Bug

Bug introduced due to unexpected behavior of third party softwares or packages, even for lack of required packages

Deployment Bug

Fixing the bugs which was causing CI failure or deployment errors.

Deprecation Bug

Removing old features and relevant codes (own or third party package induced)

Hardware Bug

Hardware related issues

Linting Bug

issues reported by linter or syntax errors

Logging Bug

Commits where log messages need modification

Network Bug

Bugs related to network parameters of OS or cluster level

New Feature

Introducing new featues

Performance Bug

Issues creating performance problem, specially long deployment time or timeout

Refactoring

Commits where code refactoring is done

Security Bug

Authentication, encryption, permission and other security bets practices related issues

Test Enhancement

Enhancing existing test cases to increase coverage

Testing Bug

Bugs for which one or more test were failining

akondrahman commented 3 years ago

@Talismanic

Thanks for the hard work. Test enhancement, testing bug, refactoring, new feature, linting can be deleted. I also think there is an opportunity to merge capacity and hardware bug to merge into one: try to use a more infrastructure sounding name for the category. Once you make the changes, I also want to see example commit messages for each category.

Talismanic commented 3 years ago

@akondrahman Bhai, I will advocate not to remove testing_bug and linting. Let me give you example of those:

Testing Bug

tests fix a typo in dev setup.yml c907ec41ae0698b7627ebcbe97f1c293611d41d7 introduced a typo. This commit fixes it. WARNING While constructing a mapping from home guits ceph ansible tests functional dev setup.yml line 21 column 9 found a duplicate dict key replace . Signed off by Guillaume Abrioux gabrioux redhat.com cherry picked from commit 2798774e965a045787ce9d05fd82e63d8329bffb

tests fix task in rhcs setup that changes vagrant box to rhel7 Signed off by Andrew Schoen aschoen redhat.com

ansible test fix up relative util import for powershell validate modules 69753 ansible test fix up relative util import for powershell validate modules Use different tactic for generic group Use python 2 and 3

Fix usage of for tests With the more recent versions of ansible we should now use is instead of the sign for the tests. This should fix it. Change Id I75025f77746c0408553e1a33719134cf6d33507a

Linting Bug

Fixes yamllint errors

Fix Ansible lint error E502 4743

Verify YAML syntax in gates Test yaml syntax is correct on new changes. Fixes current warnings and errors Change Id I5888f8e4a9d27a08506036df2c564b9f2081ccee

akondrahman commented 3 years ago

@Talismanic

  1. If you want to keep linting bug, then you need a better name like style bugs or sth. professionals and academics can relate to
  2. testing bugs itself sounds like a combination of other categories. From your example, It seems like a lot of issues are mashed up into one: like typos, and then setup ... you need to distill these things or find a better way to derive this category. Failure of a test is an outcome not the root cause. The IEEE definitions of a bug is an imperfection in the software that requires replacement or repair ... based on this IEEE definition how will you define this category?
Talismanic commented 3 years ago

@akondrahman Bhai,

  1. I have changed linting_bug to _stylebug
  2. Merged capacity, hardware and network bug into _infrabug
  3. I have removed many testing_bug, test_enhancement_bug, testing_bug, refactoring_bug, new_feature,
  4. I have introduced one new type of bug called _functionalbug

Functional Bug

The bugs which blocks the intended behavior of the code

Here is the updated link of the analysis bhaiya. Sheet name: commit_message_classification_v2

https://docs.google.com/spreadsheets/d/1aQWknk0pGBcfRX5JJ8h6UT5PbzJM7kJ-vua5QlYrr-E/edit#gid=130411827

Below is a summary:

bug_category 1 SUM of repo_type
configuration_bug 14
dependency_bug 33
deployment_bug 18
deprecation_bug 11
functional_bug 17
infra_bug 18
logging_bug 13
performance_bug 15
security_bug 8
style_bug 8
Grand Total 155
akondrahman commented 3 years ago

@Talismanic

Thanks for the hard work.

The bugs which blocks the intended behavior of the code

This definition means that deployment, dependency, performance can also become functionality bugs. How do you address this concern?

Talismanic commented 3 years ago

@akondrahman bhai

  1. Performance Bugs do not effect the functionality, rather it effects the speed of the functionality.

  2. Deployment Bugs impact the CI systems or the deployment process. Not the function

I will get back with details at night.

akondrahman commented 3 years ago

@Talismanic

How do you define function or functionality for a test? You need to make this clear. Can you please give me a sample commit message?

Talismanic commented 3 years ago

@Talismanic

How do you define function or functionality for a test? You need to make this clear. Can you please give me a sample commit message?

1 thing to clarify Bhaiya. Here functionality is the original IaC codes functionality in most of the cases.

Following are some of my consideration for your judgement

Functional Bugs

Commit Message 01

Update mongodb bootstrap tasks When deploying on hosts with PyMongo version 3.0 as found on Xenial hosts The bootstrap playbook will fail due to the following error The localhost login exception only allows the first admin account to be created To resolve this error a root user create and password setup has been added to the tasks and the required login credentials have been passed to the ceilometer and aodh tasks. Change Id Iaa29b4256dd4b3ac0774a05fc55f5b845ee57732 Signed off by Kevin Carter kevin.carter rackspace.com

Analysis: Functionality of this code is to bootstrap mongodb. However, in Xenial hosts the playbooks is failing to login to database. So technically DB was not properly bootstrapped. For fixing this, Dev Team added a task to create a root user and passed the credentials in parameters.

Commit Message 02

Fix idempotency bug in AIO bootstrap Change Id I88a563928112abf252354e546da9dce4819048e0 cherry picked from commit 13de5ffdedc136b33a300a654fc410d4a9aa277f

Analysis: Idempotency is an expected functionality of Ansible scripts. If idempotency is not working, that means there are bugs in the code.

Commit Message 03

Fix upgrades for multinode galera During an upgrade we run the upgrade in serial and we attempt to bootstrap the cluster from the galera server bootstrap node . This causes issues when there is an existing cluster that is still up. To avoid this we can simply attempt to start the cluster normally if it fails to start and join the existing cluster we can bootstrap as usual. Additionally we can make the bootstrap slightly more efficient by only running it against the bootstrap host and splitting out the galera upgrade post.yml tasks. Finally run the upgrade in serial which mirrors our approach in the integrated repo. Change Id Ic4d69f0fa75c1eea81d10a76cca9a8d9c3822094 Closes Bug 1667103

Analysis Serial Upgradation of Galera cluster is not working if an existing cluster is still up. So a bypass has been added to fix this.

akondrahman commented 3 years ago

@Talismanic

Thanks!

Please let me know what you think.

Talismanic commented 3 years ago

commit message#2 reflects idempotency and should be a category of its own, even if it is rare.

Agreed Bhaiya

commit message#1, 3 seems like infrastructure setup issues so will be infra_bug

Bhaiya for #1, this is Mongos default behavior to ensure security. First root user must be created on localhost and allowed initially only from localhost. Then that user can create other users to access from remote. So I think its not infra.

For #3, I think we can categorize this as infra. My hunch is, new cluster was not spinning the same server as previous cluster was listening on the default ports already.

Let me give you couple of more examples for your quick judgement.

Commit Message 04

Remove non kubeadm deployment 3811 Remove non kubeadm deployment More cleanup More cleanup More cleanup More cleanup Fix gitlab Try stop gce first before absent to make the delete process work More cleanup Fix bug with checking if kubeadm has already run Fix bug with checking if kubeadm has already run More fixes Fix test fix Fix gitlab checkout untill kubespray 2.8 is on quay Fixed Add upgrade path from non kubeadm to kubeadm. Revert ssl path Readd secret checking Do gitlab checks from v2.7.0 test upgrade path to 2.8.0 fix typo Fix CI jobs to kubeadm again. Fix broken hyperkube path Fix gitlab Fix rotate tokens More fixes More fixes Fix tokens

Analysis Lines which I picked are: "Fix bug with checking if kubeadm has already run", " Fix broken hyperkube path", " Fix rotate tokens". My thinking was, if kubeadm is already running, kubeadm will not spinup new k8 cluster and that is expected. Similarly tokens for users needs to be rotated.

Commit Message 05:

Better fix for different CoreOS os family facts Signed off by Bogdan Dobrelya bogdando mail.ru

Analysis Kubernetes cluster setup was showing some anomaly for CoreOS os families which needed to be fixed.

Commit Message 06

SUSE Apply workaround for mariadb 10.2 The openSUSE repository bumped the mariadb package to 10.2. However there is an upstream bug that prevents nodes from joining the cluster. The way to workaround it is to export WSREP SST OPT PORT 4444 in the systemd service file. Moreover we pull the mariadb galera subpackage which contains some necessary tools for galera clusters. Finally we drop the default configuration files which are being installed by the packages because they conflict with the ones installed by this role. hwoarang This also applies the linter fixes from https review.openstack.org c 523080 in order to make the gates happy Depends On Ia4856f36b2d106d987e3c774f31493e25a23d4b5 Link https jira.mariadb.org browse MDEV 14256 Change Id I97cf1585b2fed08f53f62a547547e422bc34fa53

Analysis MariaDB cluster set up issue.

Talismanic commented 3 years ago

@akondrahman Bhai, Here are some example of performance_bug:

Performance Bug

Commit Message 01

Use ansible facts It has come to our attention that using ansible vars that are populated with INJECT FACTS AS VARS True is not very performant. In order to be able to support setting that to off we need to update the references to use ansible facts thing instead of ansible thing . Related ansible 73654 Closes https bugzilla.redhat.com show bug.cgi id 1935406 Signed off by Alex Schultz aschultz redhat.com cherry picked from commit a7f2fa73e63e69dba2e41aaac9732397eec437c9

Analysis Intention is to speed up the operation

Commit Message 02

Refactor download role 5697 download file download containers fix push image to nodes pull if none image on host fix improve docker image tag checks. do not pull already cached images rebase fix merge conflict add support download run once when upgrade and scale cluster add some test with download run once set default values to temp flag for every download cycle add save load abilty for containerd and crio when download run once true return redefine image save load command to set docker image facts.yml move set command to set container facts ctr in containerd bin dir fix order of ctr image export arguments temporary disable download run once for containerd and crio due https github.com containerd containerd issues 4075 remove unused files fix strict yaml linter warning and errors refactor logical conditions to pull and cache container images remove comment due lint check document role remove image load on localhost because cached images are always loaded to docker on remote sites remove XXX from debug output

Analysis Downloading image in every run takes a lot of time. So basically this commit reduces the run time

Commit Message 03

Fixed parser robustness issue. Closes 61

Analysis Robustness is some kind of performance parameter

Commit Message 04

Do not discard when creating XFS loopback When creating XFS filesystems the default mkfs behavior is to discard all blocks in the filesystem. This is not necessary during AIO and disabling it with the K option will optimize execution time on certain storage mediums. Change Id Idddf89047a5efa9b4df301c731f67636976cf70e

Analysis This patch actually reduces the execution time

Commit Message 05

tests Load defaults once Apply the same logic as in the install playbook use a single playbook with tagged roles. It gives a long boring playbook but it remains readable and gets faster to run. Remove the individual playbooks.

Analysis This commit refactors the code so that playbooks can run faster

akondrahman commented 3 years ago

@Talismanic

I agree with the performance ones. But commit message 4, 5, 6 still sounds like infrastructure setup and can be mapped with infra_bug

Talismanic commented 3 years ago

@akondrahman Bhaiya, Are you considering OS(like CoreOS, Xenial) and softwares (Mongo/Maria) as a part of infra? If yes, I think your classification is agreeable.

Talismanic commented 3 years ago

@akondrahman Bhai, Next examples are for

Configuration Bug

Commit Message 01

tests fix ceph tools baseurl Signed off by Andrew Schoen aschoen redhat.com

Analysis: base_url is a config value

Commit Message 02

Update tox config and add bashate E006 E040 exceptions This patch updates the tox.ini the same bashate exceptions as are currently in the OpenStack Ansible playbook repo. It also ensures that the linters and all lint targets work appropriately and normalises the tox.ini configuration to use uniform formatting. The use of ansible.cfg is removed as there is no way of being certain which paths can be used without reverting to an ugly sed hack in the commands. This is why it is preferred to make use of environment variables which make use of tox s default substitutions instead. It s a more reliable way of achieving the goal for the purpose of gating and testing. The switch to using a git clone instead of ansible galaxy to download the plugins is due to the path spec not being able to work in Ansible 2.x. 1 1 https github.com ansible ansible issues 13563 Change Id I077c5b79a95e78fd81c33382843273893d48bbb6

Analysis: Tox configuration update

Commit Message 03

Make corrections to LXC bridge template file This change adjusts a few of the modifications made to the lxc net bridge.cfg.j2 template file in change I3c8225124a5f18db81259e1d52d0168ef52c3c17. The minus signs have been removed from if and endif blocks so that whitespace is kept intact between sections. The ordering of post up and post down commands has also been changed so that iptables rules are created before the dnsmasq service is started as they were previously. The default value of lxc net gateway has also been changed to null so that it s evaluated as expected. Its current value none is evaluated as a string. A test has been added to compare the contents of the deployed lxc bridge interface file with its expected contents. Change Id I39d7b3f40de6ac691550c11d71bb6a182b3452f4

Analysis: Template file fixing, which is a configuration type file

Commit Message 04

setup env Add registry variables for OpenStack Since ansible 2.7 the registry variables are now required for the task execution to avoid an error. msg The field environment has an invalid value which includes an undefined variable. The error was dict object has no attribute registry Change Id Ief907ea7bd3fac58fa958aa4121bc0b849bd6748

Analysis: environment variable change. Environment variables are also a kind of configuration value.

Commit Message 05

Correct data disk format when using nspawn With the change back to using a directory backed container by default the path var lib machines is no longer assumed to be running BTRFS. This change checks the container tech option and if it s nspawn sets the backend store to BTRFS otherwise the normal lxc container backing store option will be used to determine the data disk format. Change Id I8ad207d4d2b09f15cdf95b5c6c7fc8fa22449f8c Signed off by Kevin Carter kevin.carter rackspace.com

Analysis: Previously backend storage type is configured as BTRFS always. This commit allows to configure backend store type to either BTRFS or LXC based on container tech option

Commit Message 06

Work on starting the upgrade job Add a new variable UPGRADE BASEBRANCH and use that to check out a stable branch then run the bootstrap process. This will require a change to project config so that the job names will add in the branch name so we can pick it up instead of providing a value when none is present. Add a section to confd overrides for the upgrade scenario to fix the following error TASK bootstrap host Deploy user conf.d configuration fatal localhost FAILED failed true msg confd overrides scenario dict object has no attribute u upgrade Change Id Ia5fd197e81da0a1fd55327d155f181c8792bc199

Analysis: Overiding confd

Please let me know your thoughts when you can.

akondrahman commented 3 years ago

@Talismanic

If you agree with the fact that functionality bugs can be merged with existing categories then I am good. You can start the writing process.

Talismanic commented 3 years ago

@akondrahman bhai, I think I will be able to merge functional bugs in other categories.

However, I did not understand writing process part. What should I actually write Bhaiya?

akondrahman commented 3 years ago

@Talismanic first give me the final categorization of the test bug categories in a spreadsheet. Then in a separate issue I will provide instructions on writing.

Talismanic commented 3 years ago

@akondrahman Bhai, So far below is my final categorization:

IaC-Test_Script_Bug-Classification.zip

There were many commits which could not be classified due to mainly either vague commit message or too many changes in the commit message.

akondrahman commented 3 years ago

Thanks. Writing instructions available in #31