Verify configuration #713

galitf · 2018-05-02T10:17:12Z

Created lago setup command

…nteger type

…o print anything

…an't be done

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

ovirt-infra · 2018-05-02T10:17:14Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

gbenhaim · 2018-05-02T10:38:32Z

ci add to whitelist

gbenhaim

Thanks for the PR!
I've posted a few comments.

gbenhaim · 2018-05-06T12:05:27Z

lago/cmd.py

@@ -67,7 +68,6 @@
        '$PWD/.lago'
    ),
    metavar='WORKDIR',
-    type=os.path.abspath,


Why did you remove this line?

It shouldn't be removed

gbenhaim · 2018-05-06T12:10:57Z

lago/cmd.py

+    '-e',
+    dest='envs_dir',
+    help='Which directory the qemu has access permissions',
+    default="/var/lib/lago",


This switch is about where the user is going to create the lago envs.
In /var/lib/lago we store files that are being shared between the different envs.
I would use here os.curdir, or take this value from lago.conf.

gbenhaim · 2018-05-06T12:18:40Z

lago/cmd.py

+@lago.plugins.cli.cli_plugin_add_argument(
+    '--username',
+    '-u',
+    help='Which user needs to be configured',


We can add a default here:
getpass.getuser() will return the name of the current user.

gbenhaim · 2018-05-06T12:20:54Z

lago/cmd.py

+@in_lago_prefix
+@with_logging
+def do_setup(
+    prefix, username, envs_dir, verify, **kwargs


in_lago_prefix returns an object that represents a lago environment. In this case, we still don't have a lago env, so it's not needed.

gbenhaim · 2018-05-06T14:42:46Z

lago/cmd.py

+       if (os.getuid() != 0): 
+          print("Please use 'sudo', you need adminstrator permissions for configuration")
+          sys.exit(1)
+       else:


There is no need for an else here. If the first condition was true, the program exited.

gbenhaim · 2018-05-06T14:46:03Z

lago/verify_configure.py.bck

@@ -0,0 +1,293 @@
+#
+# Copyright 2014 Red Hat, Inc.


I guess that this file and all the others below are not needed?

No need
I removed it later not to upload it in my local .gitignore

gbenhaim · 2018-05-06T14:50:06Z

lago/verify_configuration.py

+        Display Lago configuration status (OK/Not-OK) Verify Lago configuration
+        """
+        if (display_list):
+            print "Configuration Status:"


A new verification step will require adding code in 4 different functions. It will be better to encapsulate all the information needed for a verification step inside an object.

gbenhaim · 2018-05-06T14:50:55Z

lago/verify_configuration.py

+    """ 
+    (exit_code,res) = exec_cmd("cat /proc/cpuinfo | egrep 'vmx|svm'")
+
+    if res == "": 


Why not using a boolean here? return res == ""

gbenhaim · 2018-05-06T14:51:32Z

lago/verify_configuration.py

+    current_permissions = stat.S_IMODE(os.lstat(path).st_mode)
+    os.chmod(path, current_permissions & NO_WRITING)
+
+def check_permissions(envs_dirs,username):


Missing space between arguments

galitf added 22 commits April 29, 2018 12:08

working on the new finctionality to check the lago setup

29c4c0e

fix the verify option

aecb7eb

added change_permissions and fix the check_permission uid/gid to be i…

5140a16

…nteger type

added a check to packages required

c23a065

added a check and change for missing packages

2026708

add setup command to lago

058d79d

working on the setup command

8fa2ee2

fixed the setup order of lines in the file

8185fb6

fix all functions to support the lago setup, add doctype

0663330

added information to the fix configuration doctype

fc43bd5

change datatype to dictionary, update functionality

8842a05

removed comments

6a4fe61

removed comment from the import area

73e46c8

used getstatusoutput instead of getoutput and os.system

67b89ab

removed copmments

4c45a46

fix the updated nested file

38fad21

remove extra printing, and fix output if pass after configuring not t…

7ed2805

…o print anything

change the message to print a link to documentation, if an auto fix c…

e202d70

…an't be done

added logger to commands

9b05fbb

fixed the string to be a parameter

e9732c2

removed test pkg

d73752a

remove spaces

2e86849

gbenhaim requested review from ifireball and eedri May 2, 2018 10:39

gbenhaim reviewed May 6, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify configuration #713

Verify configuration #713

galitf commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

gbenhaim commented May 2, 2018

gbenhaim left a comment

gbenhaim May 6, 2018

galitf May 7, 2018

gbenhaim May 6, 2018

gbenhaim May 6, 2018

gbenhaim May 6, 2018

gbenhaim May 6, 2018

gbenhaim May 6, 2018

galitf May 7, 2018

gbenhaim May 6, 2018

gbenhaim May 6, 2018

gbenhaim May 6, 2018

Verify configuration #713

Are you sure you want to change the base?

Verify configuration #713

Conversation

galitf commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

ovirt-infra commented May 2, 2018

gbenhaim commented May 2, 2018

gbenhaim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment