You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/07/29 17:48:56 UTC
[GitHub] [cloudstack] ravening opened a new pull request #4230: Enable resetting config values to default value
ravening opened a new pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230
## Description
<!--- Describe your changes in detail -->
Provide reset button to zone,cluster,domain,account,
primary and secondary storage so that config values
can be reset to the default value
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [X] 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
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
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
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-780328974
@ravening this will need changes in new UI for merge in master
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r691182035
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -851,6 +861,161 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
+ throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
+ }
+
+ String newValue = null;
+ switch (ConfigKey.Scope.valueOf(scope)) {
+ case Zone:
+ final DataCenterVO zone = _zoneDao.findById(id);
+ if (zone == null) {
+ throw new InvalidParameterValueException("unable to find zone by id " + id);
+ }
+ _dcDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Cluster:
+ final ClusterVO cluster = _clusterDao.findById(id);
+ if (cluster == null) {
+ throw new InvalidParameterValueException("unable to find cluster by id " + id);
+ }
+ ClusterDetailsVO clusterDetailsVO = _clusterDetailsDao.findDetail(id, name);
+ newValue = configKey.value().toString();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor") || name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ _clusterDetailsDao.persist(id, name, newValue);
+ } else if (clusterDetailsVO != null) {
+ _clusterDetailsDao.remove(clusterDetailsVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case StoragePool:
+ final StoragePoolVO pool = _storagePoolDao.findById(id);
+ if (pool == null) {
+ throw new InvalidParameterValueException("unable to find storage pool by id " + id);
+ }
+ _storagePoolDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Domain:
+ final DomainVO domain = _domainDao.findById(id);
+ if (domain == null) {
+ throw new InvalidParameterValueException("unable to find domain by id " + id);
+ }
+ DomainDetailVO domainDetailVO = _domainDetailsDao.findDetail(id, name);
+ if (domainDetailVO != null) {
+ _domainDetailsDao.remove(domainDetailVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueInDomain(id));
Review comment:
@ravening jenkins failed
```
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project cloud-server: Compilation failure: Compilation failure:
[ERROR] /home/jenkins/jenkins-agent/workspace/Cloudstack/cloudstack-pr-analysis/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:[902,41] error: cannot find symbol
[ERROR] symbol: class LinkedHashMap
[ERROR] location: class ConfigurationManagerImpl
[ERROR] /home/jenkins/jenkins-agent/workspace/Cloudstack/cloudstack-pr-analysis/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:[974,61] error: cannot find symbol
[ERROR] symbol: method valueInDomain(Long)
[ERROR] location: variable configKey of type ConfigKey<?>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r691131530
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -851,6 +861,161 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
+ throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
+ }
+
+ String newValue = null;
+ switch (ConfigKey.Scope.valueOf(scope)) {
+ case Zone:
+ final DataCenterVO zone = _zoneDao.findById(id);
+ if (zone == null) {
+ throw new InvalidParameterValueException("unable to find zone by id " + id);
+ }
+ _dcDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Cluster:
+ final ClusterVO cluster = _clusterDao.findById(id);
+ if (cluster == null) {
+ throw new InvalidParameterValueException("unable to find cluster by id " + id);
+ }
+ ClusterDetailsVO clusterDetailsVO = _clusterDetailsDao.findDetail(id, name);
+ newValue = configKey.value().toString();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor") || name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ _clusterDetailsDao.persist(id, name, newValue);
+ } else if (clusterDetailsVO != null) {
+ _clusterDetailsDao.remove(clusterDetailsVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case StoragePool:
+ final StoragePoolVO pool = _storagePoolDao.findById(id);
+ if (pool == null) {
+ throw new InvalidParameterValueException("unable to find storage pool by id " + id);
+ }
+ _storagePoolDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Domain:
+ final DomainVO domain = _domainDao.findById(id);
+ if (domain == null) {
+ throw new InvalidParameterValueException("unable to find domain by id " + id);
+ }
+ DomainDetailVO domainDetailVO = _domainDetailsDao.findDetail(id, name);
+ if (domainDetailVO != null) {
+ _domainDetailsDao.remove(domainDetailVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueInDomain(id));
Review comment:
It should be valueInDomain
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928181974
> > @weizhouapache i see you have committed two changes. how can i get that? i did rebase but i dont see those commits
>
> @ravening
> cannot you fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg" ?
> (assume 'origin' is the name of repository of github/ravening)
@weizhouapache i did
```
git fetch upstream
git rebase upstream/main
git checkout reset_cfg
git rebase main
```
after this i didnt get your commits
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r599576544
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/config/ResetCfgCmd.java
##########
@@ -0,0 +1,164 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.config;
+
+import org.apache.cloudstack.api.response.ImageStoreResponse;
+import org.apache.log4j.Logger;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.config.Configuration;
+
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+
+@APICommand(name = "resetConfiguration", description = "Resets a configuration. The configuration will be set to default value for global setting, and removed from account_details or domain_details for Account/Domain settings", responseObject = ConfigurationResponse.class,
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
Review comment:
since version missing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-919099874
@ravening if possible can we split the work? So that this PR is simply adding capability in UI/API (for ex. pass the default value in the list API, and let a UI button replace the default value in the text input box when a button is clicked?) to get the default value for the global setting and allow admins to reset to the default value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925648931
@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928260282
>
> @weizhouapache that is resulting in conflict in unrelated files
@ravening
it might be cause your directory is not clean, for example there are some untracked file, uncommitted changes, etc
you can remove local branch `reset-cfg`, then checkout it again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929056482
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929053243
@sureshanaparti I pushed the new ui changes. you can test it again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-930005842
> > > @ravening when I reset a global setting `account.cleanup.interval`, it does not work
> > > ```
> > > 2021-09-28 15:02:49,981 WARN [c.c.c.ConfigurationManagerImpl] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Probably the component manager where configuration variable account.cleanup.interval is defined needs to implement Configurable interface
> > > 2021-09-28 15:02:49,993 INFO [c.c.a.ApiServer] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Config parameter with name account.cleanup.interval doesn't exist
> > > ```
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > There are some configurations which are defined in Config.java, we need to take a look at these configurations.
> >
> >
> > @weizhouapache even I saw that.. some settings are not found in config store and they cant be reset but there are some others in config depot and we can reset them
>
> @ravening I think, it is better to have the reset operation for all the configuration settings, irrespective of where they are.
@sureshanaparti the reset option is available under domain/account/global scope
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-972644960
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929278109
@blueorangutan ui
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-791900949
@ravening can you fix conflicts, re-send for latest master with changes in the UI (not legacy UI)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-704243909
> > @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 ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-919712771
@ravening is it possible for you to pick those changes from the other PR to this? The other PR has conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r599568933
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/config/ResetCfgCmd.java
##########
@@ -0,0 +1,164 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.config;
+
+import org.apache.cloudstack.api.response.ImageStoreResponse;
+import org.apache.log4j.Logger;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.config.Configuration;
+
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+
+@APICommand(name = "resetConfiguration", description = "Resets a configuration. The configuration will be set to default value for global setting, and removed from account_details or domain_details for Account/Domain settings", responseObject = ConfigurationResponse.class,
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class ResetCfgCmd extends BaseCmd {
+ public static final Logger s_logger = Logger.getLogger(ResetCfgCmd.class.getName());
+ private static final String s_name = "resetconfigurationresponse";
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "the name of the configuration")
+ private String cfgName;
+
+ @Parameter(name = ApiConstants.ZONE_ID,
+ type = CommandType.UUID,
+ entityType = ZoneResponse.class,
+ description = "the ID of the Zone to reset the parameter value for corresponding zone")
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.CLUSTER_ID,
+ type = CommandType.UUID,
+ entityType = ClusterResponse.class,
+ description = "the ID of the Cluster to reset the parameter value for corresponding cluster")
+ private Long clusterId;
+
+ @Parameter(name = ApiConstants.STORAGE_ID,
+ type = CommandType.UUID,
+ entityType = StoragePoolResponse.class,
+ description = "the ID of the Storage pool to reset the parameter value for corresponding storage pool")
+ private Long storagePoolId;
+
+ @Parameter(name = ApiConstants.DOMAIN_ID,
+ type = CommandType.UUID,
+ entityType = DomainResponse.class,
+ description = "the ID of the Domain to reset the parameter value for corresponding domain")
+ private Long domainId;
+
+ @Parameter(name = ApiConstants.ACCOUNT_ID,
+ type = CommandType.UUID,
+ entityType = AccountResponse.class,
+ description = "the ID of the Account to reset the parameter value for corresponding account")
+ private Long accountId;
+
+ @Parameter(name = ApiConstants.IMAGE_STORE_UUID,
+ type = CommandType.UUID,
+ entityType = ImageStoreResponse.class,
+ description = "the ID of the Image Store to reset the parameter value for corresponding image store")
+ private Long imageStoreId;
+
+ /////////////////////////////////////////////////////
+ /////////////////// Accessors ///////////////////////
+ /////////////////////////////////////////////////////
+
+ public String getCfgName() {
+ return cfgName;
+ }
+
+ public Long getZoneId() {
+ return zoneId;
+ }
+
+ public Long getClusterId() {
+ return clusterId;
+ }
+
+ public Long getStoragepoolId() {
+ return storagePoolId;
+ }
+
+ public Long getDomainId() {
+ return domainId;
+ }
+
+ public Long getAccountId() {
+ return accountId;
+ }
+
+ public Long getImageStoreId() {
+ return imageStoreId;
+ }
+
+ /////////////////////////////////////////////////////
+ /////////////// API Implementation///////////////////
+ /////////////////////////////////////////////////////
+
+ @Override
+ public String getCommandName() {
+ return s_name;
+ }
+
+ @Override
+ public long getEntityOwnerId() {
+ return Account.ACCOUNT_ID_SYSTEM;
+ }
+
+ @Override
+ public void execute() {
+ Pair<Configuration, String> cfg = _configService.resetConfiguration(this);
+ if (cfg != null) {
+ ConfigurationResponse response = _responseGenerator.createConfigurationResponse(cfg.first());
+ response.setResponseName(getCommandName());
+ if (getZoneId() != null) {
+ response.setScope("zone");
+ }
+ if (getClusterId() != null) {
+ response.setScope("cluster");
+ }
+ if (getStoragepoolId() != null) {
+ response.setScope("storagepool");
Review comment:
`storagepool` => ConfigKey.Scope.StoragePool.toString();
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-926570564
Ping @ravening is this ready?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-927709515
@ravening Tested reset configuration from API (using cmk) / UI, it is working fine. Noticed the following issues with UI, Can you please check. Thanks.
- Global settings UI doesn't have reset configuration button
<img width="1007" alt="Global_Settings" src="https://user-images.githubusercontent.com/12028987/134886881-e7901d04-4c78-48a1-bd00-05f1d837e98d.png">
- Scope-level UI shows extra icon/button
Settings @ Primary Storage =>
<img width="981" alt="PrimaryStorage_Settings" src="https://user-images.githubusercontent.com/12028987/134886924-95457c21-77b6-46be-a2ff-6139c13a810a.png">
Settings @ Accounts =>
<img width="983" alt="Account_Settings" src="https://user-images.githubusercontent.com/12028987/134886956-06c6de80-ca8d-4d8b-ba68-5a6a41a94428.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-927709515
@ravening Tested reset configuration from API (using cmk) / UI, it is working fine. Noticed the following issues with UI, Can you please check. Thanks.
- Global settings UI doesn't have reset configuration button
<img width="1007" alt="Global_Settings" src="https://user-images.githubusercontent.com/12028987/134886881-e7901d04-4c78-48a1-bd00-05f1d837e98d.png">
- Scope-level UI shows extra icon/button
`Settings @ Primary Storage =>`
<img width="981" alt="PrimaryStorage_Settings" src="https://user-images.githubusercontent.com/12028987/134886924-95457c21-77b6-46be-a2ff-6139c13a810a.png">
`Settings @ Accounts =>`
<img width="983" alt="Account_Settings" src="https://user-images.githubusercontent.com/12028987/134886956-06c6de80-ca8d-4d8b-ba68-5a6a41a94428.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-975377154
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-972645239
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-901066605
> lease review again
@ravening please fix build failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003361027
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-717931677
> > > @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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-778020150
@ravening can you fix conflicts, re-send for latest master with changes in the UI (not legacy UI)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929927962
> @ravening when I reset a global setting `account.cleanup.interval`, it does not work
>
> ```
> 2021-09-28 15:02:49,981 WARN [c.c.c.ConfigurationManagerImpl] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Probably the component manager where configuration variable account.cleanup.interval is defined needs to implement Configurable interface
> 2021-09-28 15:02:49,993 INFO [c.c.a.ApiServer] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Config parameter with name account.cleanup.interval doesn't exist
> ```
>
> There are some configurations which are defined in Config.java, we need to take a look at these configurations.
@weizhouapache even I saw that.. some settings are not found in config store and they cant be reset but there are some others in config depot and we can reset them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934194476
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1506
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-926986863
<b>Trillian test result (tid-2207)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31860 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4230-t2207-kvm-centos7.zip
Smoke tests completed. 90 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-926688417
@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928202921
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-717930382
> > @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?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti closed pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti closed pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-669059135
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-704215760
@ravening marking this 4.16 as it is waiting on dependencies. please argue if you disagree.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-669067308
Packaging result: ✖centos7 ✖debian. JID-1655
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-673992840
> @ravening can you check Travis failures
@rhtyd the build will fail since some functions are missing.
Those functions are defined in another pr
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-930075794
@ravening can you plase rebase against latest main branch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925648533
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925738251
@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929056155
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925645485
@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r691131530
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -851,6 +861,161 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
+ throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
+ }
+
+ String newValue = null;
+ switch (ConfigKey.Scope.valueOf(scope)) {
+ case Zone:
+ final DataCenterVO zone = _zoneDao.findById(id);
+ if (zone == null) {
+ throw new InvalidParameterValueException("unable to find zone by id " + id);
+ }
+ _dcDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Cluster:
+ final ClusterVO cluster = _clusterDao.findById(id);
+ if (cluster == null) {
+ throw new InvalidParameterValueException("unable to find cluster by id " + id);
+ }
+ ClusterDetailsVO clusterDetailsVO = _clusterDetailsDao.findDetail(id, name);
+ newValue = configKey.value().toString();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor") || name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ _clusterDetailsDao.persist(id, name, newValue);
+ } else if (clusterDetailsVO != null) {
+ _clusterDetailsDao.remove(clusterDetailsVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case StoragePool:
+ final StoragePoolVO pool = _storagePoolDao.findById(id);
+ if (pool == null) {
+ throw new InvalidParameterValueException("unable to find storage pool by id " + id);
+ }
+ _storagePoolDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Domain:
+ final DomainVO domain = _domainDao.findById(id);
+ if (domain == null) {
+ throw new InvalidParameterValueException("unable to find domain by id " + id);
+ }
+ DomainDetailVO domainDetailVO = _domainDetailsDao.findDetail(id, name);
+ if (domainDetailVO != null) {
+ _domainDetailsDao.remove(domainDetailVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueInDomain(id));
Review comment:
It should be valueInDomain
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti closed pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti closed pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-870550367
ping @ravening can you please resolve the outstanding comments, and fix the code conflicts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1002985439
@rohityadavcloud a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003907214
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-919106665
> @ravening if possible can we split the work? So that this PR is simply adding capability in UI/API (for ex. pass the default value in the list API, and let a UI button replace the default value in the text input box when a button is clicked?) to get the default value for the global setting and allow admins to reset to the default value.
@rhtyd this pr only has the functionality to reset the values to default values. Since the scope involves account, domain and global setting level, some part of the code is present in another pr which has to be merged first to fix the build errors
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r712073453
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -851,6 +861,161 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
+ throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
+ }
+
+ String newValue = null;
+ switch (ConfigKey.Scope.valueOf(scope)) {
+ case Zone:
+ final DataCenterVO zone = _zoneDao.findById(id);
+ if (zone == null) {
+ throw new InvalidParameterValueException("unable to find zone by id " + id);
+ }
+ _dcDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Cluster:
+ final ClusterVO cluster = _clusterDao.findById(id);
+ if (cluster == null) {
+ throw new InvalidParameterValueException("unable to find cluster by id " + id);
+ }
+ ClusterDetailsVO clusterDetailsVO = _clusterDetailsDao.findDetail(id, name);
+ newValue = configKey.value().toString();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor") || name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ _clusterDetailsDao.persist(id, name, newValue);
+ } else if (clusterDetailsVO != null) {
+ _clusterDetailsDao.remove(clusterDetailsVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case StoragePool:
+ final StoragePoolVO pool = _storagePoolDao.findById(id);
+ if (pool == null) {
+ throw new InvalidParameterValueException("unable to find storage pool by id " + id);
+ }
+ _storagePoolDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Domain:
+ final DomainVO domain = _domainDao.findById(id);
+ if (domain == null) {
+ throw new InvalidParameterValueException("unable to find domain by id " + id);
+ }
+ DomainDetailVO domainDetailVO = _domainDetailsDao.findDetail(id, name);
+ if (domainDetailVO != null) {
+ _domainDetailsDao.remove(domainDetailVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueInDomain(id));
Review comment:
@weizhouapache yes because, few part of the code are present in another pr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925644959
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r466268971
##########
File path: engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java
##########
@@ -103,18 +103,31 @@ public void persist(long clusterId, Map<String, String> details) {
expunge(sc);
for (Map.Entry<String, String> detail : details.entrySet()) {
+ String name = detail.getKey();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor")) {
+ name = "cpuOvercommitRatio";
+ }
+ if (name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ name = "memoryOvercommitRatio";
+ }
Review comment:
You can run the integration test cases using `nosetests --with-marvin --marvin-config=<cfg file> test_reset_configuration_settings.py`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-704236633
> @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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-912176348
Ping @ravening
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-927912780
@weizhouapache i see you have committed two changes. how can i get that? i did rebase but i dont see those commits
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929279155
@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-972836788
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 1731
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-975470696
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934133584
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003453442
<b>Trillian test result (tid-2760)</b>
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
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003970459
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003314452
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929283273
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4230 (SL-JID-705)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-669072481
@rhtyd dont merge this as this depends on #4215
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-922636141
ping @ravening can you close the outstanding comments on this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r714712808
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -851,6 +861,159 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ private ParamCountPair getParamCount(Map<String, Long> scopeMap) {
+ Long id = null;
+ int paramCount = 0;
+ String scope = ConfigKey.Scope.Global.toString();
+
+ for (var entry : scopeMap.entrySet()) {
+ if (entry.getValue() != null) {
+ id = entry.getValue();
+ scope = entry.getKey();
+ paramCount++;
+ }
+ }
+
+ return new ParamCountPair(id, paramCount, scope);
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = "";
+ Map<String, Long> scopeMap = new LinkedHashMap<>();
Review comment:
order is not important, right ? build failures caused by unimported LinkedHashMap class.
```suggestion
Map<String, Long> scopeMap = new HashMap<>();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-918882773
> Ping @ravening can you fix build failure? I like this feature, would be really useful for users.
@rhtyd this depends on #4215 and thats builds are failing. As @weizhouapache mentioned, shall i merge that pr to this or should I keep it separate?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929317792
@ravening
when I reset a global setting `account.cleanup.interval`, it does not work
```
2021-09-28 15:02:49,981 WARN [c.c.c.ConfigurationManagerImpl] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Probably the component manager where configuration variable account.cleanup.interval is defined needs to implement Configurable interface
2021-09-28 15:02:49,993 INFO [c.c.a.ApiServer] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Config parameter with name account.cleanup.interval doesn't exist
```
There are some configurations which are defined in Config.java, we need to take a look at these configurations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934246253
Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1511
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-975471560
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-975570161
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r465668970
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -814,6 +824,161 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
+ throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
+ }
+
+ String newValue = null;
+ switch (ConfigKey.Scope.valueOf(scope)) {
+ case Zone:
+ final DataCenterVO zone = _zoneDao.findById(id);
Review comment:
And here, you could extract any of this case to an smalll method, add some documentation and unit test for this news methods.
##########
File path: engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java
##########
@@ -103,18 +103,31 @@ public void persist(long clusterId, Map<String, String> details) {
expunge(sc);
for (Map.Entry<String, String> detail : details.entrySet()) {
+ String name = detail.getKey();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor")) {
+ name = "cpuOvercommitRatio";
+ }
+ if (name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ name = "memoryOvercommitRatio";
+ }
String value = detail.getValue();
if ("password".equals(detail.getKey())) {
value = DBEncryptionUtil.encrypt(value);
}
- ClusterDetailsVO vo = new ClusterDetailsVO(clusterId, detail.getKey(), value);
+ ClusterDetailsVO vo = new ClusterDetailsVO(clusterId, name, value);
persist(vo);
}
txn.commit();
}
@Override
public void persist(long clusterId, String name, String value) {
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor")) {
Review comment:
If you extract an method you can reuse it here.
##########
File path: engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java
##########
@@ -103,18 +103,31 @@ public void persist(long clusterId, Map<String, String> details) {
expunge(sc);
for (Map.Entry<String, String> detail : details.entrySet()) {
+ String name = detail.getKey();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor")) {
+ name = "cpuOvercommitRatio";
+ }
+ if (name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ name = "memoryOvercommitRatio";
+ }
Review comment:
I think you could extract this lines into an new method.
Maybe extract those Strings into constants
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r599569879
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/config/ResetCfgCmd.java
##########
@@ -0,0 +1,164 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.config;
+
+import org.apache.cloudstack.api.response.ImageStoreResponse;
+import org.apache.log4j.Logger;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.config.Configuration;
+
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+
+@APICommand(name = "resetConfiguration", description = "Resets a configuration. The configuration will be set to default value for global setting, and removed from account_details or domain_details for Account/Domain settings", responseObject = ConfigurationResponse.class,
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class ResetCfgCmd extends BaseCmd {
+ public static final Logger s_logger = Logger.getLogger(ResetCfgCmd.class.getName());
+ private static final String s_name = "resetconfigurationresponse";
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "the name of the configuration")
+ private String cfgName;
+
+ @Parameter(name = ApiConstants.ZONE_ID,
+ type = CommandType.UUID,
+ entityType = ZoneResponse.class,
+ description = "the ID of the Zone to reset the parameter value for corresponding zone")
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.CLUSTER_ID,
+ type = CommandType.UUID,
+ entityType = ClusterResponse.class,
+ description = "the ID of the Cluster to reset the parameter value for corresponding cluster")
+ private Long clusterId;
+
+ @Parameter(name = ApiConstants.STORAGE_ID,
+ type = CommandType.UUID,
+ entityType = StoragePoolResponse.class,
+ description = "the ID of the Storage pool to reset the parameter value for corresponding storage pool")
+ private Long storagePoolId;
+
+ @Parameter(name = ApiConstants.DOMAIN_ID,
+ type = CommandType.UUID,
+ entityType = DomainResponse.class,
+ description = "the ID of the Domain to reset the parameter value for corresponding domain")
+ private Long domainId;
+
+ @Parameter(name = ApiConstants.ACCOUNT_ID,
+ type = CommandType.UUID,
+ entityType = AccountResponse.class,
+ description = "the ID of the Account to reset the parameter value for corresponding account")
+ private Long accountId;
+
+ @Parameter(name = ApiConstants.IMAGE_STORE_UUID,
+ type = CommandType.UUID,
+ entityType = ImageStoreResponse.class,
+ description = "the ID of the Image Store to reset the parameter value for corresponding image store")
+ private Long imageStoreId;
+
+ /////////////////////////////////////////////////////
+ /////////////////// Accessors ///////////////////////
+ /////////////////////////////////////////////////////
+
+ public String getCfgName() {
+ return cfgName;
+ }
+
+ public Long getZoneId() {
+ return zoneId;
+ }
+
+ public Long getClusterId() {
+ return clusterId;
+ }
+
+ public Long getStoragepoolId() {
+ return storagePoolId;
+ }
+
+ public Long getDomainId() {
+ return domainId;
+ }
+
+ public Long getAccountId() {
+ return accountId;
+ }
+
+ public Long getImageStoreId() {
+ return imageStoreId;
+ }
+
+ /////////////////////////////////////////////////////
+ /////////////// API Implementation///////////////////
+ /////////////////////////////////////////////////////
+
+ @Override
+ public String getCommandName() {
+ return s_name;
+ }
+
+ @Override
+ public long getEntityOwnerId() {
+ return Account.ACCOUNT_ID_SYSTEM;
+ }
+
+ @Override
+ public void execute() {
+ Pair<Configuration, String> cfg = _configService.resetConfiguration(this);
+ if (cfg != null) {
+ ConfigurationResponse response = _responseGenerator.createConfigurationResponse(cfg.first());
+ response.setResponseName(getCommandName());
+ if (getZoneId() != null) {
+ response.setScope("zone");
+ }
+ if (getClusterId() != null) {
+ response.setScope("cluster");
+ }
+ if (getStoragepoolId() != null) {
+ response.setScope("storagepool");
+ }
+ if (getDomainId() != null) {
+ response.setScope("domain");
Review comment:
Use scope defined in ConfigKey, ConfigKey.Scope.Domain.toString() here. Same for the other scopes values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003360787
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003907907
@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-669058577
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934279715
@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934199830
@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934132905
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929982366
@ravening
since #4215 has been merged into master, you need to rebase this PR with main branch, and make some necessary changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003313836
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-975998594
<b>Trillian test result (tid-2574)</b>
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
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-997619870
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-717835573
@ravening is this still in progress, opened as a draft?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-901031907
@sureshanaparti @rhtyd please review again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-901031907
@sureshanaparti @rhtyd please review again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-927709515
@ravening Tested reset configuration from API (using cmk) / UI, it is working fine. Noticed the following issues with UI, Can you please check. Thanks.
- Global settings UI doesn't have reset configuration button
<img width="1007" alt="Global_Settings" src="https://user-images.githubusercontent.com/12028987/134886881-e7901d04-4c78-48a1-bd00-05f1d837e98d.png">
- Scope-level UI shows extra icon/button
Settings @ Primary Storage =>
<img width="981" alt="PrimaryStorage_Settings" src="https://user-images.githubusercontent.com/12028987/134886924-95457c21-77b6-46be-a2ff-6139c13a810a.png">
Settings @ Accounts =>
<img width="983" alt="Account_Settings" src="https://user-images.githubusercontent.com/12028987/134886956-06c6de80-ca8d-4d8b-ba68-5a6a41a94428.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-927709515
@ravening Tested reset configuration from API (using cmk) / UI, it is working fine. Noticed the following issues with UI, Can you please check. Thanks.
- Global settings UI doesn't have reset configuration button
<img width="1007" alt="Global_Settings" src="https://user-images.githubusercontent.com/12028987/134886881-e7901d04-4c78-48a1-bd00-05f1d837e98d.png">
- Scope-level UI shows extra icon/button
Settings @ Primary Storage =>
<img width="981" alt="PrimaryStorage_Settings" src="https://user-images.githubusercontent.com/12028987/134886924-95457c21-77b6-46be-a2ff-6139c13a810a.png">
Settings @ Accounts =>
<img width="983" alt="Account_Settings" src="https://user-images.githubusercontent.com/12028987/134886956-06c6de80-ca8d-4d8b-ba68-5a6a41a94428.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd closed pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925667731
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r599579603
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/config/ResetCfgCmd.java
##########
@@ -0,0 +1,164 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.config;
+
+import org.apache.cloudstack.api.response.ImageStoreResponse;
+import org.apache.log4j.Logger;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.config.Configuration;
+
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+
+@APICommand(name = "resetConfiguration", description = "Resets a configuration. The configuration will be set to default value for global setting, and removed from account_details or domain_details for Account/Domain settings", responseObject = ConfigurationResponse.class,
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class ResetCfgCmd extends BaseCmd {
+ public static final Logger s_logger = Logger.getLogger(ResetCfgCmd.class.getName());
+ private static final String s_name = "resetconfigurationresponse";
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "the name of the configuration")
+ private String cfgName;
+
+ @Parameter(name = ApiConstants.ZONE_ID,
+ type = CommandType.UUID,
+ entityType = ZoneResponse.class,
+ description = "the ID of the Zone to reset the parameter value for corresponding zone")
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.CLUSTER_ID,
+ type = CommandType.UUID,
+ entityType = ClusterResponse.class,
+ description = "the ID of the Cluster to reset the parameter value for corresponding cluster")
+ private Long clusterId;
+
+ @Parameter(name = ApiConstants.STORAGE_ID,
+ type = CommandType.UUID,
+ entityType = StoragePoolResponse.class,
+ description = "the ID of the Storage pool to reset the parameter value for corresponding storage pool")
+ private Long storagePoolId;
+
+ @Parameter(name = ApiConstants.DOMAIN_ID,
+ type = CommandType.UUID,
+ entityType = DomainResponse.class,
+ description = "the ID of the Domain to reset the parameter value for corresponding domain")
+ private Long domainId;
+
+ @Parameter(name = ApiConstants.ACCOUNT_ID,
+ type = CommandType.UUID,
+ entityType = AccountResponse.class,
+ description = "the ID of the Account to reset the parameter value for corresponding account")
+ private Long accountId;
+
+ @Parameter(name = ApiConstants.IMAGE_STORE_UUID,
Review comment:
IMAGE_STORE_ID, in sync with other params ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r464419774
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -814,6 +824,163 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString())) {
+ if (!configScope.contains(scope) && !(configScope.contains(ConfigKey.Scope.Account.toString()) && scope.equals(ConfigKey.Scope.Domain.toString()))) {
Review comment:
@DaanHoogland this was the extra check which got added because of https://github.com/apache/cloudstack/pull/4215
I have removed it now but still this pr depends on the above one
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-673991749
@ravening can you check Travis failures
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-930075794
@ravening can you plase rebase against latest main branch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache edited a comment on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache edited a comment on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928237309
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929963108
@weizhouapache @sureshanaparti @ravening , maybe this is a good time to phase out the old config conpletely and move the remaining config items to appropriate modules? I'm not sure how big an effort it will still be, but it will be a good cleanup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925737745
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd closed pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-927940084
> @ravening Tested reset configuration from API (using cmk) / UI, it is working fine. Noticed the following issues with UI, Can you please check. Thanks.
>
> * Global settings UI doesn't have reset configuration button
>
> <img alt="Global_Settings" width="1007" src="https://user-images.githubusercontent.com/12028987/134886881-e7901d04-4c78-48a1-bd00-05f1d837e98d.png">
>
> * Scope-level UI shows extra icon/button
>
> `Settings @ Primary Storage =>`
> <img alt="PrimaryStorage_Settings" width="981" src="https://user-images.githubusercontent.com/12028987/134886924-95457c21-77b6-46be-a2ff-6139c13a810a.png">
>
> `Settings @ Accounts =>`
> <img alt="Account_Settings" width="983" src="https://user-images.githubusercontent.com/12028987/134886956-06c6de80-ca8d-4d8b-ba68-5a6a41a94428.png">
@sureshanaparti looks like something messed up.. i will look into it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-914911524
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-673935750
@ravening is this ready for review? (PR still in draft)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r685858268
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -851,6 +861,161 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
+ throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
+ }
+
+ String newValue = null;
+ switch (ConfigKey.Scope.valueOf(scope)) {
+ case Zone:
+ final DataCenterVO zone = _zoneDao.findById(id);
+ if (zone == null) {
+ throw new InvalidParameterValueException("unable to find zone by id " + id);
+ }
+ _dcDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Cluster:
+ final ClusterVO cluster = _clusterDao.findById(id);
+ if (cluster == null) {
+ throw new InvalidParameterValueException("unable to find cluster by id " + id);
+ }
+ ClusterDetailsVO clusterDetailsVO = _clusterDetailsDao.findDetail(id, name);
+ newValue = configKey.value().toString();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor") || name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ _clusterDetailsDao.persist(id, name, newValue);
+ } else if (clusterDetailsVO != null) {
+ _clusterDetailsDao.remove(clusterDetailsVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case StoragePool:
+ final StoragePoolVO pool = _storagePoolDao.findById(id);
+ if (pool == null) {
+ throw new InvalidParameterValueException("unable to find storage pool by id " + id);
+ }
+ _storagePoolDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Domain:
+ final DomainVO domain = _domainDao.findById(id);
+ if (domain == null) {
+ throw new InvalidParameterValueException("unable to find domain by id " + id);
+ }
+ DomainDetailVO domainDetailVO = _domainDetailsDao.findDetail(id, name);
+ if (domainDetailVO != null) {
+ _domainDetailsDao.remove(domainDetailVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueInDomain(id));
Review comment:
```suggestion
optionalValue = Optional.ofNullable(configKey.valueIn(id));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-901640169
> > lease review again
>
> @ravening please fix build failures.
@ravening since this depends on #4215, can you include #4215 in this pr ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928246849
> > > > > > @weizhouapache i see you have committed two changes. how can i get that? i did rebase but i dont see those commits
>
> > > > >
>
> > > > >
>
> > > > > @ravening
>
> > > > > cannot you fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg" ?
>
> > > > > (assume 'origin' is the name of repository of github/ravening)
>
> > > >
>
> > > >
>
> > > > @weizhouapache i did
>
> > > > ```
>
> > > > git fetch upstream
>
> > > > git rebase upstream/main
>
> > > > git checkout reset_cfg
>
> > > > git rebase main
>
> > > > ```
>
> > > >
>
> > > >
>
> > > >
>
> > > >
>
> > > >
>
> > > >
>
> > > >
>
> > > >
>
> > > >
>
> > > >
>
> > > >
>
> > > > after this i didnt get your commits
>
> > >
>
> > >
>
> > > git rebase upstream/reset_cfg
>
> > > not `main`
>
> >
>
> > @weizhouapache getting error
>
> >
>
> > ```
>
> > git rebase upstream/reset_cfg ─╯
>
> > fatal: invalid upstream 'upstream/reset_cfg'
>
> >
>
> > git remote -v ─╯
>
> > origin https://github.com/ravening/cloudstack.git (fetch)
>
> > origin https://github.com/ravening/cloudstack.git (push)
>
> > upstream https://github.com/apache/cloudstack.git (fetch)
>
> > upstream https://github.com/apache/cloudstack.git (push)
>
> > ```
>
>
>
> @ravening please see my previous reply.
>
>
>
> ```
>
> fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg"
>
> (assume 'origin' is the name of repository of github/ravening)
>
> ```
>
>
>
>
@weizhouapache that is resulting in conflict in unrelated files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929056482
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-927968019
> @weizhouapache i see you have committed two changes. how can i get that? i did rebase but i dont see those commits
@ravening
cannot you fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg" ?
(assume 'origin' is the name of repository of github/ravening)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003944975
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2064
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003970172
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r776994235
##########
File path: test/integration/smoke/test_reset_configuration_settings.py
##########
@@ -0,0 +1,375 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.lib.utils import (validateList,
+ cleanup_resources)
+from marvin.lib.base import (Account,
+ Configurations,
+ Domain,
+ Cluster,
+ StoragePool)
+from marvin.lib.common import (get_domain,
+ get_zone)
+
+class TestRestConfigurationSettings(cloudstackTestCase):
+ @classmethod
+ def setUpClass(self):
+ self.testClient = super(
+ TestRestConfigurationSettings,
+ self).getClsTestClient()
+ self.apiclient = self.testClient.getApiClient()
+ self.testdata = self.testClient.getParsedTestDataConfig()
+
+ # Get Zone, Domain
+ self.domain = get_domain(self.apiclient)
+ self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests())
+ self._cleanup = []
+ return
+
+ @classmethod
+ def tearDownClass(self):
+ try:
+ # Cleanup resources used
+ cleanup_resources(self.apiclient, self._cleanup)
+ except Exception as e:
+ raise Exception("Warning: Exception during cleanup : %s" % e)
+ return
+
+ def setUp(self):
+ self.apiclient = self.testClient.getApiClient()
+ self.dbclient = self.testClient.getDbConnection()
+ self.cleanup = []
+
+ return
+
+ def tearDown(self):
+ try:
+ # Clean up
+ cleanup_resources(self.apiclient, self.cleanup)
+ except Exception as e:
+ raise Exception("Warning: Exception during cleanup : %s" % e)
+ return
Review comment:
```suggestion
def tearDown(self):
super(TestRestConfigurationSettings, self).tearDown()
return
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-975420334
Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1760
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r462541950
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -814,6 +824,163 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString())) {
+ if (!configScope.contains(scope) && !(configScope.contains(ConfigKey.Scope.Account.toString()) && scope.equals(ConfigKey.Scope.Domain.toString()))) {
Review comment:
so, the scope provided must contain (¿not be equal to?) be the scope the key is provided for *AND* may not contain both account and be equal to domain? I don't get that last part.
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -814,6 +824,163 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString())) {
Review comment:
👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-914910629
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929003300
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928237309
> > > > > @weizhouapache i see you have committed two changes. how can i get that? i did rebase but i dont see those commits
> > > >
> > > >
> > > > @ravening
> > > > cannot you fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg" ?
> > > > (assume 'origin' is the name of repository of github/ravening)
> > >
> > >
> > > @weizhouapache i did
> > > ```
> > > git fetch upstream
> > > git rebase upstream/main
> > > git checkout reset_cfg
> > > git rebase main
> > > ```
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > after this i didnt get your commits
> >
> >
> > git rebase upstream/reset_cfg
> > not `main`
>
> @weizhouapache getting error
>
> ```
> git rebase upstream/reset_cfg ─╯
> fatal: invalid upstream 'upstream/reset_cfg'
>
> git remote -v ─╯
> origin https://github.com/ravening/cloudstack.git (fetch)
> origin https://github.com/ravening/cloudstack.git (push)
> upstream https://github.com/apache/cloudstack.git (fetch)
> upstream https://github.com/apache/cloudstack.git (push)
> ```
@ravening please see my previous reply.
```
cannot you fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg" ?
(assume 'origin' is the name of repository of github/ravening)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929932960
> > @ravening when I reset a global setting `account.cleanup.interval`, it does not work
> > ```
> > 2021-09-28 15:02:49,981 WARN [c.c.c.ConfigurationManagerImpl] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Probably the component manager where configuration variable account.cleanup.interval is defined needs to implement Configurable interface
> > 2021-09-28 15:02:49,993 INFO [c.c.a.ApiServer] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Config parameter with name account.cleanup.interval doesn't exist
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > There are some configurations which are defined in Config.java, we need to take a look at these configurations.
>
> @weizhouapache even I saw that.. some settings are not found in config store and they cant be reset but there are some others in config depot and we can reset them
@ravening I think, it is better to have the reset operation for all the configuration settings, irrespective of where they are.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929927962
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-997620052
@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-926734001
@weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925773933
@weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934279276
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934199830
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1002985263
Ping @ravening is this ready for review/testing, needs any rebasing or fixing?
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-673936256
@rhtyd yes it's ready for review. It's marked as draft because it can't be merged unless other is merged
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-918848446
Ping @ravening can you fix build failure? I like this feature, would be really useful for users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r599585486
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -835,6 +845,161 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
Review comment:
most of the code here is redundant with the existing code for update configuration. move common to private methods and re-use them here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929963108
@weizhouapache @sureshanaparti @ravening , maybe this is a good time to phase out the old config conpletely and move the remaining config items to appropriate modules? I'm not sure how big an effort it will still be, but it will be a good cleanup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934198746
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-901066605
> lease review again
@ravening please fix build failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-717901125
> @ravening is this still in progress, opened as a draft?
@rhtyd yes ready for review but depends on #4215
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-975520707
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1761
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-975570896
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-976375567
@ravening @sureshanaparti is this ready for merge now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925169004
I agree with @weizhouapache's proposal. @ravening can you please address that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929982366
@ravening
since #4215 has been merged into master, you need to rebase this PR with main branch, and make some necessary changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934321469
@ravening
I think you need to revert https://github.com/apache/cloudstack/commit/5c188736ea09ecd04c26e2992b8da8bb1ac07f4b, and part of https://github.com/apache/cloudstack/commit/8bff4888183db43283ecc2dc985165da2b1b4d2b, right ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache edited a comment on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache edited a comment on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928260282
>
> @weizhouapache that is resulting in conflict in unrelated files
@ravening
it might be cause your directory is not clean, for example there are some untracked file, uncommitted changes, etc
you can remove local branch `reset_cfg`, then checkout it again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003346237
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2054
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928202921
> > > @weizhouapache i see you have committed two changes. how can i get that? i did rebase but i dont see those commits
> >
> >
> > @ravening
> > cannot you fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg" ?
> > (assume 'origin' is the name of repository of github/ravening)
>
> @weizhouapache i did
>
> ```
> git fetch upstream
> git rebase upstream/main
> git checkout reset_cfg
> git rebase main
> ```
>
> after this i didnt get your commits
git rebase upstream/reset_cfg
not `main`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-926687971
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache edited a comment on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache edited a comment on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928237309
> > > > > @weizhouapache i see you have committed two changes. how can i get that? i did rebase but i dont see those commits
> > > >
> > > >
> > > > @ravening
> > > > cannot you fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg" ?
> > > > (assume 'origin' is the name of repository of github/ravening)
> > >
> > >
> > > @weizhouapache i did
> > > ```
> > > git fetch upstream
> > > git rebase upstream/main
> > > git checkout reset_cfg
> > > git rebase main
> > > ```
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > after this i didnt get your commits
> >
> >
> > git rebase upstream/reset_cfg
> > not `main`
>
> @weizhouapache getting error
>
> ```
> git rebase upstream/reset_cfg ─╯
> fatal: invalid upstream 'upstream/reset_cfg'
>
> git remote -v ─╯
> origin https://github.com/ravening/cloudstack.git (fetch)
> origin https://github.com/ravening/cloudstack.git (push)
> upstream https://github.com/apache/cloudstack.git (fetch)
> upstream https://github.com/apache/cloudstack.git (push)
> ```
@ravening please see my previous reply.
```
fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg"
(assume 'origin' is the name of repository of github/ravening)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-926712317
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1392
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925758800
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1381
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti closed pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti closed pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-997657994
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland merged pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-922975382
@ravening
comparing with #4215, I think this had less risk and is more acceptable.
I suggest to remove #4215 from this pr. let's work on this pr and merge it at first.
what's your opinion ? @nvazquez @sureshanaparti
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti removed a comment on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti removed a comment on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934198746
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r691182035
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -851,6 +861,161 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
+ throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
+ }
+
+ String newValue = null;
+ switch (ConfigKey.Scope.valueOf(scope)) {
+ case Zone:
+ final DataCenterVO zone = _zoneDao.findById(id);
+ if (zone == null) {
+ throw new InvalidParameterValueException("unable to find zone by id " + id);
+ }
+ _dcDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Cluster:
+ final ClusterVO cluster = _clusterDao.findById(id);
+ if (cluster == null) {
+ throw new InvalidParameterValueException("unable to find cluster by id " + id);
+ }
+ ClusterDetailsVO clusterDetailsVO = _clusterDetailsDao.findDetail(id, name);
+ newValue = configKey.value().toString();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor") || name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ _clusterDetailsDao.persist(id, name, newValue);
+ } else if (clusterDetailsVO != null) {
+ _clusterDetailsDao.remove(clusterDetailsVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case StoragePool:
+ final StoragePoolVO pool = _storagePoolDao.findById(id);
+ if (pool == null) {
+ throw new InvalidParameterValueException("unable to find storage pool by id " + id);
+ }
+ _storagePoolDetailsDao.removeDetail(id, name);
+ optionalValue = Optional.ofNullable(configKey.valueIn(id));
+ newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
+ break;
+
+ case Domain:
+ final DomainVO domain = _domainDao.findById(id);
+ if (domain == null) {
+ throw new InvalidParameterValueException("unable to find domain by id " + id);
+ }
+ DomainDetailVO domainDetailVO = _domainDetailsDao.findDetail(id, name);
+ if (domainDetailVO != null) {
+ _domainDetailsDao.remove(domainDetailVO.getId());
+ }
+ optionalValue = Optional.ofNullable(configKey.valueInDomain(id));
Review comment:
@ravening jenkins failed
```
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project cloud-server: Compilation failure: Compilation failure:
[ERROR] /home/jenkins/jenkins-agent/workspace/Cloudstack/cloudstack-pr-analysis/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:[902,41] error: cannot find symbol
[ERROR] symbol: class LinkedHashMap
[ERROR] location: class ConfigurationManagerImpl
[ERROR] /home/jenkins/jenkins-agent/workspace/Cloudstack/cloudstack-pr-analysis/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:[974,61] error: cannot find symbol
[ERROR] symbol: method valueInDomain(Long)
[ERROR] location: variable configKey of type ConfigKey<?>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-914931589
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 1159
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-923810454
> @ravening
> comparing with #4215, I think this had less risk and is more acceptable.
> I suggest to remove #4215 from this pr. let's work on this pr and merge it at first.
>
> what's your opinion ? @nvazquez @sureshanaparti
@weizhouapache agree with the riskier part as this introduces new API and no much changes in the existing config code/flow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928228073
> > > > @weizhouapache i see you have committed two changes. how can i get that? i did rebase but i dont see those commits
> > >
> > >
> > > @ravening
> > > cannot you fetch the latest commits by "git fetch origin", then checkout to local branch "reset_cfg" and rebase it by "git rebase origin/reset_cfg" ?
> > > (assume 'origin' is the name of repository of github/ravening)
> >
> >
> > @weizhouapache i did
> > ```
> > git fetch upstream
> > git rebase upstream/main
> > git checkout reset_cfg
> > git rebase main
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > after this i didnt get your commits
>
> git rebase upstream/reset_cfg
> not `main`
@weizhouapache getting error
```
git rebase upstream/reset_cfg ─╯
fatal: invalid upstream 'upstream/reset_cfg'
git remote -v ─╯
origin https://github.com/ravening/cloudstack.git (fetch)
origin https://github.com/ravening/cloudstack.git (push)
upstream https://github.com/apache/cloudstack.git (fetch)
upstream https://github.com/apache/cloudstack.git (push)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-870550367
ping @ravening can you please resolve the outstanding comments, and fix the code conflicts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-934301517
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1513
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r599575395
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/config/ResetCfgCmd.java
##########
@@ -0,0 +1,164 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.config;
+
+import org.apache.cloudstack.api.response.ImageStoreResponse;
+import org.apache.log4j.Logger;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.config.Configuration;
+
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+
+@APICommand(name = "resetConfiguration", description = "Resets a configuration. The configuration will be set to default value for global setting, and removed from account_details or domain_details for Account/Domain settings", responseObject = ConfigurationResponse.class,
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class ResetCfgCmd extends BaseCmd {
+ public static final Logger s_logger = Logger.getLogger(ResetCfgCmd.class.getName());
+ private static final String s_name = "resetconfigurationresponse";
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "the name of the configuration")
Review comment:
add _validations = {ApiArgValidator.NotNullOrEmpty}_ here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-926733514
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929932960
> > @ravening when I reset a global setting `account.cleanup.interval`, it does not work
> > ```
> > 2021-09-28 15:02:49,981 WARN [c.c.c.ConfigurationManagerImpl] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Probably the component manager where configuration variable account.cleanup.interval is defined needs to implement Configurable interface
> > 2021-09-28 15:02:49,993 INFO [c.c.a.ApiServer] (qtp1840903588-16:ctx-b50154f5 ctx-d20a74f0) (logid:3ee4ec69) Config parameter with name account.cleanup.interval doesn't exist
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > There are some configurations which are defined in Config.java, we need to take a look at these configurations.
>
> @weizhouapache even I saw that.. some settings are not found in config store and they cant be reset but there are some others in config depot and we can reset them
@ravening I think, it is better to have the reset operation for all the configuration settings, irrespective of where they are.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929091263
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1423
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-929003300
> > @weizhouapache that is resulting in conflict in unrelated files
>
> @ravening
> it might be cause your directory is not clean, for example there are some untracked file, uncommitted changes, etc
>
> you can remove local branch `reset_cfg`, then checkout it again.
yes, @weizhouapache is right,
I think you better remove local branches `reset_cfg` and `main`. next
```
git fetch origin
git fetch upstream
git checkout -b main upstream/main
git checkout -b reset_cfg origin/reset_cfg
git rebase main
```
your original plan to,
```
git fetch upstream
git rebase upstream/main
git checkout reset_cfg
git rebase main
```
, should have worked unless you had leftovers that were in the way, or not the latest of either repo, which might have caused a conflict.
Have a look at `git rebase --help`, it is easy to mess up a bit, but hard to cause real damage, don't worry(, be happy).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-925773622
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-928181974
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-997640243
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1952
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r776994109
##########
File path: test/integration/smoke/test_reset_configuration_settings.py
##########
@@ -0,0 +1,375 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.lib.utils import (validateList,
+ cleanup_resources)
+from marvin.lib.base import (Account,
+ Configurations,
+ Domain,
+ Cluster,
+ StoragePool)
+from marvin.lib.common import (get_domain,
+ get_zone)
+
+class TestRestConfigurationSettings(cloudstackTestCase):
+ @classmethod
+ def setUpClass(self):
+ self.testClient = super(
+ TestRestConfigurationSettings,
+ self).getClsTestClient()
+ self.apiclient = self.testClient.getApiClient()
+ self.testdata = self.testClient.getParsedTestDataConfig()
+
+ # Get Zone, Domain
+ self.domain = get_domain(self.apiclient)
+ self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests())
+ self._cleanup = []
+ return
+
+ @classmethod
+ def tearDownClass(self):
+ try:
+ # Cleanup resources used
+ cleanup_resources(self.apiclient, self._cleanup)
+ except Exception as e:
+ raise Exception("Warning: Exception during cleanup : %s" % e)
+ return
Review comment:
```suggestion
def tearDownClass(cls):
super(TestRestConfigurationSettings, cls).tearDownClass()
return
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1003013412
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 2043
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-1004282468
<b>Trillian test result (tid-2767)</b>
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
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4230: Enable resetting config values to default value
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#issuecomment-975376299
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org