Skip to content

Enable resetting config values to default value - #4230

Merged
DaanHoogland merged 4 commits into
apache:4.16from
ravening:reset_cfg
Jan 3, 2022
Merged

DaanHoogland merged 4 commits into
apache:4.16from
ravening:reset_cfg

Conversation

@ravening

@ravening ravening commented Jul 29, 2020 •

Copy link
Copy Markdown
Member

Description

Provide reset button to zone,cluster,domain,account,
primary and secondary storage so that config values
can be reset to the default value

Depends on #4215

Types of changes

  • Enhancement (improves an existing feature and functionality)

Screenshots (if appropriate):

Reset button has been provided under zone,cluster, domain, account, primary storage and secondary storage level

Screenshot 2021-03-23 at 14 07 26

How Has This Been Tested?

Tested throguh cmk also

(local) mgt01 > list configurations name=allow.user.expunge.recover.vm accountid=4048bfec-c28e-11ea-9a43-0617bc003377
{
  "configuration": [
    {
      "category": "Advanced",
      "description": "Determines whether users can expunge or recover their vm",
      "isdynamic": true,
      "name": "allow.user.expunge.recover.vm",
      "scope": "account",
      "value": "false"
    }
  ],
  "count": 1
}
(local) mgt01 > update configuration name=allow.user.expunge.recover.vm accountid=4048bfec-c28e-11ea-9a43-0617bc003377 value=true
{
  "configuration": {
    "category": "Advanced",
    "description": "Determines whether users can expunge or recover their vm",
    "isdynamic": true,
    "name": "allow.user.expunge.recover.vm",
    "scope": "account",
    "value": "true"
  }
}

(local) mgt01 > reset configuration name=allow.user.expunge.recover.vm accountid=4048bfec-c28e-11ea-9a43-0617bc003377
{
  "configuration": {
    "category": "Advanced",
    "description": "Determines whether users can expunge or recover their vm",
    "isdynamic": true,
    "name": "allow.user.expunge.recover.vm",
    "scope": "account",
    "value": "false"
  }
}
(local) mgt01 > list configurations name=allow.user.expunge.recover.vm accountid=4048bfec-c28e-11ea-9a43-0617bc003377
{
  "configuration": [
    {
      "category": "Advanced",
      "description": "Determines whether users can expunge or recover their vm",
      "isdynamic": true,
      "name": "allow.user.expunge.recover.vm",
      "scope": "account",
      "value": "false"
    }
  ],
  "count": 1
}

Run integration test case using

nosetests --with-marvin --marvin-config=<cfg file> test_reset_configuration_settings.py

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one question and a general remark: scopes are a enum, so I think string compares using contains() should be avoided.

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testing required, good looking

@yadvr

yadvr commented Aug 5, 2020

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✖centos7 ✖debian. JID-1655

@ravening

ravening commented Aug 5, 2020

Copy link
Copy Markdown
Member Author

@rhtyd dont merge this as this depends on #4215

@DaanHoogland
DaanHoogland marked this pull request as draft August 5, 2020 09:58

@RodrigoDLopez RodrigoDLopez left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ravening
Nice feature, I think you could break some code into smaller pieces, add some documentation and unit tests. Small method is easy to make unit tests , and with unit tests, future new codes do not need to be manually tested every time
I did not run manual tests yet, but I will do it soon, into the next days, I hope.

@yadvr

yadvr commented Aug 14, 2020

Copy link
Copy Markdown
Member

@ravening is this ready for review? (PR still in draft)

@ravening

Copy link
Copy Markdown
Member Author

@rhtyd yes it's ready for review. It's marked as draft because it can't be merged unless other is merged

@yadvr

yadvr commented Aug 14, 2020

Copy link
Copy Markdown
Member

@ravening can you check Travis failures

@ravening

Copy link
Copy Markdown
Member Author

@ravening can you check Travis failures

@rhtyd the build will fail since some functions are missing.
Those functions are defined in another pr

ustcweizhou added a commit to ravening/cloudstack that referenced this pull request Oct 6, 2020
ustcweizhou added a commit to ravening/cloudstack that referenced this pull request Oct 6, 2020
… under domain settings"

This reverts commit 0ecdaeb.
@DaanHoogland DaanHoogland added this to the 4.16.0.0 milestone Oct 6, 2020
@DaanHoogland

Copy link
Copy Markdown
Contributor

@ravening marking this 4.16 as it is waiting on dependencies. please argue if you disagree.

@ravening

ravening commented Oct 6, 2020

Copy link
Copy Markdown
Member Author

@ravening marking this 4.16 as it is waiting on dependencies. please argue if you disagree.

I prefer if it can go in 4.15.

@DaanHoogland

Copy link
Copy Markdown
Contributor

@ravening marking this 4.16 as it is waiting on dependencies. please argue if you disagree.

I prefer if it can go in 4.15.

I assumed as much but than can you push for any prerequisite work as well, please? ( I want/prefer is not the argument I was hoping for ;)

@yadvr

yadvr commented Oct 28, 2020

Copy link
Copy Markdown
Member

@ravening is this still in progress, opened as a draft?

@ravening
ravening marked this pull request as ready for review October 28, 2020 12:29
@ravening

Copy link
Copy Markdown
Member Author

@ravening is this still in progress, opened as a draft?

@rhtyd yes ready for review but depends on #4215

@DaanHoogland

Copy link
Copy Markdown
Contributor

@ravening is this still in progress, opened as a draft?

@rhtyd yes ready for review but depends on #4215

@ravening is the code depending on #4215 or you use-case?

@ravening

Copy link
Copy Markdown
Member Author

@ravening is this still in progress, opened as a draft?

@rhtyd yes ready for review but depends on #4215

@ravening is the code depending on #4215 or you use-case?

@DaanHoogland yes it depends on other pr as some functions are defined there

@yadvr

yadvr commented Feb 12, 2021

Copy link
Copy Markdown
Member

@ravening can you fix conflicts, re-send for latest master with changes in the UI (not legacy UI)

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1761

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2574)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31189 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4230-t2574-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test Result Time (s) Test File

@DaanHoogland

Copy link
Copy Markdown
Contributor

@ravening @sureshanaparti is this ready for merge now?

@sureshanaparti

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1952

@sureshanaparti

Copy link
Copy Markdown
Contributor

@blueorangutan test

@yadvr

yadvr commented Dec 30, 2021

Copy link
Copy Markdown
Member

Ping @ravening is this ready for review/testing, needs any rebasing or fixing?
@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rohityadavcloud a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 2043

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2054

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2760)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31687 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4230-t2760-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test Result Time (s) Test File

@sureshanaparti

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2064

@sureshanaparti

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2767)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31131 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4230-t2767-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test Result Time (s) Test File

@DaanHoogland
DaanHoogland merged commit 2bd1dc1 into apache:4.16 Jan 3, 2022
@weizhouapache weizhouapache mentioned this pull request Mar 28, 2022
2 of 10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

10 participants