Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Provide Custom Info as Pillar data #3093

Merged
merged 6 commits into from Mar 30, 2021
Merged

Provide Custom Info as Pillar data #3093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0646322
Provide Custom Info as Pillar
nadvornik Jan 6, 2021
58e8bc8
Test for Custom Info in Pillar
nadvornik Jan 7, 2021
3fb0a03
Fix UpdateCustomDataAction
nadvornik Jan 12, 2021
4efac65
Simplify writing of individual pillar files
nadvornik Jan 13, 2021
3ec086c
Put Custom Info in separate pillar file
nadvornik Mar 12, 2021
7e4bb9d
Update changelogs
nadvornik Mar 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions java/code/src/com/redhat/rhn/domain/server/ServerFactory.java
View file
Open in desktop
Expand Up @@ -40,6 +40,7 @@
import com.redhat.rhn.manager.system.UpdateBaseChannelCommand;

import com.suse.manager.model.maintenance.MaintenanceSchedule;
import com.suse.manager.webui.services.pillar.MinionPillarManager;

import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
Expand Down Expand Up @@ -115,6 +116,10 @@ public static void removeCustomDataValues(Server server) {
SINGLETON.removeObject(value);
}
server.getCustomDataValues().clear();
server.asMinionServer().ifPresent(minion -> {
MinionPillarManager.INSTANCE.generatePillar(minion, false,
MinionPillarManager.PillarSubset.CUSTOM_INFO);
});
}

/**
Expand All @@ -129,6 +134,10 @@ public static void removeCustomDataValue(Server server, CustomDataKey key) {
if (value != null) {
SINGLETON.removeObject(value);
}
server.asMinionServer().ifPresent(minion -> {
MinionPillarManager.INSTANCE.generatePillar(minion, false,
MinionPillarManager.PillarSubset.CUSTOM_INFO);
});
}

/**
Expand Down Expand Up @@ -682,7 +691,13 @@ public static void removeCustomKey(CustomDataKey keyIn) {
List<CustomDataValue> values = lookupCustomDataValues(keyIn);
for (Iterator itr = values.iterator(); itr.hasNext();) {
CustomDataValue value = (CustomDataValue) itr.next();
Server server = value.getServer();
server.getCustomDataValues().remove(value);
SINGLETON.removeObject(value);
server.asMinionServer().ifPresent(minion -> {
MinionPillarManager.INSTANCE.generatePillar(minion, false,
MinionPillarManager.PillarSubset.CUSTOM_INFO);
});
}

SINGLETON.removeObject(keyIn);
Expand Down
14 changes: 10 additions & 4 deletions java/code/src/com/redhat/rhn/frontend/action/systems/sdc/UpdateCustomDataAction.java
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@
import com.redhat.rhn.frontend.struts.RhnValidationHelper;
import com.redhat.rhn.frontend.struts.StrutsDelegate;
import com.redhat.rhn.manager.system.SystemManager;
import com.suse.manager.webui.services.pillar.MinionPillarManager;

import org.apache.struts.action.ActionForm;
import org.apache.struts.action.ActionForward;
Expand Down Expand Up @@ -123,16 +124,21 @@ public ActionForward execute(ActionMapping mapping,
return getStrutsDelegate().forwardParams(
mapping.findForward(RhnHelper.DEFAULT_FORWARD), params);
}
server.addCustomDataValue(key.getLabel(), (String)form.get(VAL_PARAM), user);
if (cdv == null) {
cdv = new CustomDataValue();
cdv.setKey(key);
cdv.setValue((String)form.get(VAL_PARAM));
cdv.setCreated(new Date());
cdv.setCreator(user);
}
cdv.setValue((String)form.get(VAL_PARAM));
cdv.setModified(new Date());
cdv.setCreated(new Date());
cdv.setCreator(user);
cdv.setLastModifier(user);
server.addCustomDataValue(cdv);
server.asMinionServer().ifPresent(minion -> {
MinionPillarManager.INSTANCE.generatePillar(minion, false,
MinionPillarManager.PillarSubset.CUSTOM_INFO);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit confused by the logic from lines above this. It seems to me that addCustomDataValue already creates a new CustomDataValue instance and sets it if null, and now we have the logic repeated here.

I would not mind in the context of this PR normally, but here we are adding code which is clearly back-end (pillar generation) to a front-end class (an Action), and was wondering where it would be placed best.

Would it be possible to fold the code above and the pillar generation inside of addCustomDataValue - either to be left in Server or moved into ServerFactory?


request.setAttribute(VAL_PARAM, form.get(VAL_PARAM));
return getStrutsDelegate().forwardParams(mapping.findForward("updated"),
params);
Expand Down
6 changes: 6 additions & 0 deletions java/code/src/com/redhat/rhn/frontend/xmlrpc/system/SystemHandler.java
View file
Open in desktop
Expand Up @@ -170,6 +170,7 @@
import com.redhat.rhn.manager.token.ActivationKeyManager;
import com.redhat.rhn.taskomatic.TaskomaticApi;

import com.suse.manager.webui.services.pillar.MinionPillarManager;
import com.suse.manager.webui.utils.gson.BootstrapParameters;

import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -2013,6 +2014,11 @@ public int setCustomValues(User loggedInUser, Integer sid, Map<String, String> v
}
}

server.asMinionServer().ifPresent(minion -> {
MinionPillarManager.INSTANCE.generatePillar(minion, false,
MinionPillarManager.PillarSubset.CUSTOM_INFO);
});

// If we skipped any keys, we need to throw an exception and let the user know.
if (skippedKeys.size() > 0) {
// We need to throw an exception. Append each undefined key to the
Expand Down
44 changes: 43 additions & 1 deletion java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java
View file
Open in desktop
Expand Up @@ -151,7 +151,11 @@
import org.jmock.imposters.ByteBuddyClassImposteriser;
import org.jmock.integration.junit3.JUnit3Mockery;
import org.jmock.lib.concurrent.Synchroniser;
import org.yaml.snakeyaml.Yaml;

import java.io.FileInputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
Expand All @@ -170,6 +174,9 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import static com.suse.manager.webui.services.SaltConstants.PILLAR_DATA_FILE_PREFIX;
import static com.suse.manager.webui.services.SaltConstants.PILLAR_DATA_FILE_EXT;

public class SystemHandlerTest extends BaseHandlerTestCase {

private TaskomaticApi taskomaticApi = new TaskomaticApi();
Expand Down Expand Up @@ -1073,8 +1080,27 @@ private String getLongTestString() {
return longString.toString();
}

private Map<String, Object> readCustomInfoPillar(MinionServer minion) throws Exception {
Path filePath = tmpPillarRoot.resolve(
PILLAR_DATA_FILE_PREFIX + "_" +
minion.getMinionId() + "_custom_info." +
PILLAR_DATA_FILE_EXT);

assertTrue(Files.exists(filePath));

Map<String, Object> map;
try (FileInputStream fi = new FileInputStream(filePath.toFile())) {
map = new Yaml().loadAs(fi, Map.class);
}

assertTrue(map.containsKey("custom_info"));
map = (Map<String, Object>)map.get("custom_info");

return map;
}

public void testCustomDataValues() throws Exception {
Server server = ServerFactoryTest.createTestServer(admin);
MinionServer server = MinionServerFactoryTest.createTestMinionServer(admin);
CustomDataKey testKey = CustomDataKeyTest.createTestCustomDataKey(admin);

// setCustomValues
Expand Down Expand Up @@ -1105,6 +1131,11 @@ public void testCustomDataValues() throws Exception {
assertEquals(val2, val.getValue());
assertEquals(1, setResult);

Map<String, Object> pillar = readCustomInfoPillar(server);

assertTrue(pillar.containsKey(keyLabel));
assertEquals(val2, pillar.get(keyLabel));

// try to set custom values with some undefined keys
valuesToSet.put(fooKey, val1);
try {
Expand Down Expand Up @@ -1138,6 +1169,10 @@ public void testCustomDataValues() throws Exception {
val = server.getCustomDataValue(testKey);
assertNotNull(val);

pillar = readCustomInfoPillar(server);
assertTrue(pillar.containsKey(keyLabel));
assertEquals(val2, pillar.get(keyLabel));

result = handler.getCustomValues(admin, server.getId().intValue());
assertEquals(1, result.size());

Expand All @@ -1151,6 +1186,13 @@ public void testCustomDataValues() throws Exception {
assertEquals(1, setResult);
val = server.getCustomDataValue(testKey);
assertNull(val);

Path filePath = tmpPillarRoot.resolve(
PILLAR_DATA_FILE_PREFIX + "_" +
server.getMinionId() + "_custom_info." +
PILLAR_DATA_FILE_EXT);

assertFalse(Files.exists(filePath));
}

public void testListUserSystems() throws Exception {
Expand Down
11 changes: 2 additions & 9 deletions java/code/src/com/redhat/rhn/frontend/xmlrpc/test/BaseHandlerTestCase.java
View file
Open in desktop
Expand Up @@ -23,9 +23,7 @@
import com.redhat.rhn.testing.TestUtils;
import com.redhat.rhn.testing.UserTestUtils;

import com.suse.manager.webui.services.pillar.MinionGeneralPillarGenerator;
import com.suse.manager.webui.services.pillar.MinionGroupMembershipPillarGenerator;
import com.suse.manager.webui.services.pillar.MinionPillarFileManager;
import com.suse.manager.webui.services.pillar.MinionPillarManager;
import com.suse.manager.webui.services.SaltStateGeneratorService;

import java.nio.file.Files;
Expand All @@ -48,10 +46,6 @@ public class BaseHandlerTestCase extends RhnBaseTestCase {
protected String satAdminKey;
protected Path tmpPillarRoot;
protected Path tmpSaltRoot;
protected MinionPillarFileManager minionGroupMembershipPillarFileManager =
new MinionPillarFileManager(new MinionGroupMembershipPillarGenerator());
protected MinionPillarFileManager minionGeneralPillarFileManager =
new MinionPillarFileManager(new MinionGeneralPillarGenerator());

@Override
public void setUp() throws Exception {
Expand Down Expand Up @@ -84,8 +78,7 @@ public void setUp() throws Exception {

tmpPillarRoot = Files.createTempDirectory("pillar");
tmpSaltRoot = Files.createTempDirectory("salt");
minionGroupMembershipPillarFileManager.setPillarDataPath(tmpPillarRoot.toAbsolutePath());
minionGeneralPillarFileManager.setPillarDataPath(tmpPillarRoot.toAbsolutePath());
MinionPillarManager.INSTANCE.setPillarDataPath(tmpPillarRoot.toAbsolutePath());
SaltStateGeneratorService.INSTANCE.setSuseManagerStatesFilesRoot(tmpSaltRoot
.toAbsolutePath());
Files.createDirectory(tmpSaltRoot.resolve(SALT_CONFIG_STATES_DIR));
Expand Down
17 changes: 3 additions & 14 deletions java/code/src/com/redhat/rhn/manager/system/ServerGroupManager.java
View file
Open in desktop
Expand Up @@ -31,8 +31,7 @@
import com.redhat.rhn.domain.user.UserFactory;

import com.suse.manager.webui.services.SaltStateGeneratorService;
import com.suse.manager.webui.services.pillar.MinionGroupMembershipPillarGenerator;
import com.suse.manager.webui.services.pillar.MinionPillarFileManager;
import com.suse.manager.webui.services.pillar.MinionPillarManager;
import com.suse.utils.Opt;

import org.apache.log4j.Logger;
Expand All @@ -54,17 +53,6 @@ public class ServerGroupManager {
/** Logger */
private static final Logger LOG = Logger.getLogger(ServerGroupManager.class);

private MinionPillarFileManager minionGroupMembershipPillarFileManager =
new MinionPillarFileManager(new MinionGroupMembershipPillarGenerator());

/**
* Only used for unit tests.
* @param pillarFileManagerIn to set
*/
public void setMinionGroupMembershipPillarFileManager(MinionPillarFileManager pillarFileManagerIn) {
this.minionGroupMembershipPillarFileManager = pillarFileManagerIn;
}

/**
* Lookup a ServerGroup by ID and organization.
* @param id Server group id
Expand Down Expand Up @@ -350,7 +338,8 @@ public void addServers(ServerGroup sg, Collection<Server> servers, User loggedIn
*/
public void updatePillarAfterGroupUpdateForServers(Collection<Server> servers) {
servers.stream().map(server -> server.asMinionServer()).flatMap(Opt::stream)
.forEach(this.minionGroupMembershipPillarFileManager::updatePillarFile);
.forEach(s -> MinionPillarManager.INSTANCE.generatePillar(s, false,
MinionPillarManager.PillarSubset.GROUP_MEMBERSHIP));
}

/**
Expand Down
6 changes: 3 additions & 3 deletions java/code/src/com/redhat/rhn/manager/system/entitling/SystemEntitler.java
View file
Open in desktop
Expand Up @@ -45,8 +45,7 @@
import com.suse.manager.webui.services.iface.SaltApi;
import com.suse.manager.webui.services.iface.VirtManager;
import com.suse.manager.webui.services.impl.SaltSSHService;
import com.suse.manager.webui.services.pillar.MinionPillarFileManager;
import com.suse.manager.webui.services.pillar.MinionVirtualizationPillarGenerator;
import com.suse.manager.webui.services.pillar.MinionPillarManager;

import org.apache.log4j.Logger;

Expand Down Expand Up @@ -138,7 +137,8 @@ else if (EntitlementManager.OSIMAGE_BUILD_HOST.equals(ent)) {
if (wasVirtEntitled && !EntitlementManager.VIRTUALIZATION.equals(ent) ||
!wasVirtEntitled && EntitlementManager.VIRTUALIZATION.equals(ent)) {
this.updateLibvirtEngine(minion);
new MinionPillarFileManager(new MinionVirtualizationPillarGenerator()).updatePillarFile(minion);
MinionPillarManager.INSTANCE.generatePillar(minion, false,
MinionPillarManager.PillarSubset.VIRTUALIZATION);
}

if (EntitlementManager.MONITORING.equals(ent)) {
Expand Down
6 changes: 3 additions & 3 deletions java/code/src/com/redhat/rhn/manager/system/entitling/SystemUnentitler.java
View file
Open in desktop
Expand Up @@ -25,8 +25,7 @@

import com.suse.manager.webui.services.iface.MonitoringManager;
import com.suse.manager.webui.services.iface.VirtManager;
import com.suse.manager.webui.services.pillar.MinionPillarFileManager;
import com.suse.manager.webui.services.pillar.MinionVirtualizationPillarGenerator;
import com.suse.manager.webui.services.pillar.MinionPillarManager;

import org.apache.log4j.Logger;

Expand Down Expand Up @@ -101,7 +100,8 @@ public void removeServerEntitlement(Server server, Entitlement ent) {

if (EntitlementManager.VIRTUALIZATION.equals(ent)) {
virtManager.updateLibvirtEngine(s);
new MinionPillarFileManager(new MinionVirtualizationPillarGenerator()).updatePillarFile(s);
MinionPillarManager.INSTANCE.generatePillar(s, false,
MinionPillarManager.PillarSubset.VIRTUALIZATION);
}
});
}
Expand Down
11 changes: 2 additions & 9 deletions java/code/src/com/redhat/rhn/testing/BaseTestCaseWithUser.java
View file
Open in desktop
Expand Up @@ -19,9 +19,7 @@
import com.redhat.rhn.domain.user.User;

import com.suse.manager.webui.services.SaltStateGeneratorService;
import com.suse.manager.webui.services.pillar.MinionGeneralPillarGenerator;
import com.suse.manager.webui.services.pillar.MinionGroupMembershipPillarGenerator;
import com.suse.manager.webui.services.pillar.MinionPillarFileManager;
import com.suse.manager.webui.services.pillar.MinionPillarManager;

import org.apache.commons.io.FileUtils;

Expand All @@ -39,10 +37,6 @@ public abstract class BaseTestCaseWithUser extends RhnBaseTestCase {
private boolean committed = false;
protected Path tmpPillarRoot;
protected Path tmpSaltRoot;
protected MinionPillarFileManager minionGroupMembershipPillarFileManager =
new MinionPillarFileManager(new MinionGroupMembershipPillarGenerator());
protected MinionPillarFileManager minionGeneralPillarFileManager =
new MinionPillarFileManager(new MinionGeneralPillarGenerator());

/**
* {@inheritDoc}
Expand All @@ -55,8 +49,7 @@ public void setUp() throws Exception {
KickstartDataTest.setupTestConfiguration(user);
tmpPillarRoot = Files.createTempDirectory("pillar");
tmpSaltRoot = Files.createTempDirectory("salt");
minionGroupMembershipPillarFileManager.setPillarDataPath(tmpPillarRoot.toAbsolutePath());
minionGeneralPillarFileManager.setPillarDataPath(tmpPillarRoot.toAbsolutePath());
MinionPillarManager.INSTANCE.setPillarDataPath(tmpPillarRoot.toAbsolutePath());
SaltStateGeneratorService.INSTANCE.setSuseManagerStatesFilesRoot(tmpSaltRoot
.toAbsolutePath());
Files.createDirectory(tmpSaltRoot.resolve(SALT_CONFIG_STATES_DIR));
Expand Down
14 changes: 2 additions & 12 deletions java/code/src/com/redhat/rhn/testing/JMockBaseTestCaseWithUser.java
View file
Open in desktop
Expand Up @@ -14,14 +14,11 @@
*/
package com.redhat.rhn.testing;

import com.redhat.rhn.GlobalInstanceHolder;
import com.redhat.rhn.domain.kickstart.test.KickstartDataTest;
import com.redhat.rhn.domain.user.User;

import com.suse.manager.webui.services.SaltStateGeneratorService;
import com.suse.manager.webui.services.pillar.MinionGeneralPillarGenerator;
import com.suse.manager.webui.services.pillar.MinionGroupMembershipPillarGenerator;
import com.suse.manager.webui.services.pillar.MinionPillarFileManager;
import com.suse.manager.webui.services.pillar.MinionPillarManager;

import org.apache.commons.io.FileUtils;

Expand All @@ -38,10 +35,6 @@ public abstract class JMockBaseTestCaseWithUser extends RhnJmockBaseTestCase {
protected User user;
protected Path tmpPillarRoot;
protected Path tmpSaltRoot;
protected MinionPillarFileManager minionGroupMembershipPillarFileManager =
new MinionPillarFileManager(new MinionGroupMembershipPillarGenerator());
protected MinionPillarFileManager minionGeneralPillarFileManager =
new MinionPillarFileManager(new MinionGeneralPillarGenerator());

/**
* {@inheritDoc}
Expand All @@ -54,13 +47,10 @@ public void setUp() throws Exception {
KickstartDataTest.setupTestConfiguration(user);
tmpPillarRoot = Files.createTempDirectory("pillar");
tmpSaltRoot = Files.createTempDirectory("salt");
minionGroupMembershipPillarFileManager.setPillarDataPath(tmpPillarRoot.toAbsolutePath());
minionGeneralPillarFileManager.setPillarDataPath(tmpPillarRoot.toAbsolutePath());
MinionPillarManager.INSTANCE.setPillarDataPath(tmpPillarRoot.toAbsolutePath());
SaltStateGeneratorService.INSTANCE.setSuseManagerStatesFilesRoot(tmpSaltRoot
.toAbsolutePath());
Files.createDirectory(tmpSaltRoot.resolve(SALT_CONFIG_STATES_DIR));
GlobalInstanceHolder.SERVER_GROUP_MANAGER
.setMinionGroupMembershipPillarFileManager(minionGroupMembershipPillarFileManager);
}

/**
Expand Down