Skip to content

Commit

Permalink
Merge pull request #9534 from mcalmer/improve-channel-change
Browse files Browse the repository at this point in the history 
Improve channel change
  • Loading branch information
@mcalmer
mcalmer authored Dec 10, 2024
2 parents 614c2e4 + 8682459 commit b3cf9ef
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 32 deletions.
27 changes: 24 additions & 3 deletions java/code/src/com/redhat/rhn/frontend/xmlrpc/system/SystemHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
import com.redhat.rhn.frontend.xmlrpc.InvalidPackageArchException;
import com.redhat.rhn.frontend.xmlrpc.InvalidPackageException;
import com.redhat.rhn.frontend.xmlrpc.InvalidParameterException;
import com.redhat.rhn.frontend.xmlrpc.InvalidParentChannelException;
import com.redhat.rhn.frontend.xmlrpc.InvalidProfileLabelException;
import com.redhat.rhn.frontend.xmlrpc.InvalidSystemException;
import com.redhat.rhn.frontend.xmlrpc.MethodInvalidParamException;
Expand Down Expand Up @@ -564,8 +565,15 @@ public long scheduleChangeChannels(User loggedInUser, Integer sid, String baseCh
* @apidoc.doc Schedule an action to change the channels of the given system. Works for both traditional
* and Salt systems.
* This method accepts labels for the base and child channels.
* If the user provides an empty string for the channelLabel, the current base channel and
* all child channels will be removed from the system.
* If the user provides an empty string for the baseChannelLabel and an empty list for the childLabels,
* the current base channel and all child channels will be removed from the system.
* If the user provides only a different baseChannelLabel and an empty list for childLabels,
* the new base channel is assigned to the system and we search for compatible child channels for the assigned one.
* If the base channel stay empty, all the child channels from the list should be compatible with the currently
* assigned base channel. The currently assigned child channels are exchanged with the channels provided in
* the childLabels list.
* When both baseChannelLabel and childLabels are provided, the compatibility is checked, and the system
* gets these new set of channels assigned.
*
* @apidoc.param #session_key()
* @apidoc.param #array_single("int", "sids")
Expand Down Expand Up @@ -596,7 +604,7 @@ public List<Long> scheduleChangeChannels(User loggedInUser, List<Integer> sids,
.collect(toSet());

for (Server s : servers) {
if (baseChannel.map(Channel::getChannelArch)
if (baseChannel.isPresent() && baseChannel.map(Channel::getChannelArch)
.filter(arch -> arch.isCompatible(s.getServerArch()))
.isEmpty()) {
throw new InvalidChannelException();
Expand Down Expand Up @@ -625,6 +633,19 @@ public List<Long> scheduleChangeChannels(User loggedInUser, List<Integer> sids,
.map(cid -> ChannelFactory.lookupByIdAndUser(cid, loggedInUser))
.toList();

// consistent check
long bid = baseChannel.map(Channel::getId).orElse(-1L);
for (Server s : servers) {
if (bid == -1L) {
bid = s.getBaseChannel().getId();
}
for (Channel cch : childChannels) {
if (!cch.getParentChannel().getId().equals(bid)) {
throw new InvalidParentChannelException();
}
}
}

try {
Set<Action> action = ActionChainManager.scheduleSubscribeChannelsAction(loggedInUser,
serverIds,
Expand Down
54 changes: 30 additions & 24 deletions java/code/src/com/redhat/rhn/manager/system/SystemManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3629,6 +3629,13 @@ public static void addMinionInfoToServer(Long sid, String minionId) {
* the {@link UpdateBaseChannelCommand} and {@link UpdateChildChannelsCommand}.
* This method regenerate the Tokens and Pillar Data synchronous, but does not
* trigger a 'state.apply' for channels.
* <br/>
* To unsubscribe from all channels, set baseChannel to empty and provide an empty list of child channels
* To switch the base channel and let it find compatible child channels to the currently subscribed child channels,
* just provide the new base channel and provide an empty list for the child channels
* If no base channel is provided, the list of child channels should have the currently assigned base channel as
* parent.
* If both, base and child channels are provided, the system will be changed to use the defined channels
*
* @param user the user changing the channels
* @param server the server for which to change channels
Expand All @@ -3640,30 +3647,29 @@ public void updateServerChannels(User user,
Server server,
Optional<Channel> baseChannel,
Collection<Channel> childChannels) {
long baseChannelId =
baseChannel.map(Channel::getId).orElse(-1L);

// if there's no base channel present the there are no child channels to set
List<Long> childChannelIds = baseChannel.isPresent() ?
childChannels.stream().map(Channel::getId).collect(Collectors.toList()) :
emptyList();

UpdateBaseChannelCommand baseChannelCommand =
new UpdateBaseChannelCommand(
user,
server,
baseChannelId);

UpdateChildChannelsCommand childChannelsCommand =
new UpdateChildChannelsCommand(
user,
server,
childChannelIds);

baseChannelCommand.skipChannelChangedEvent(true);
baseChannelCommand.store();
childChannelsCommand.skipChannelChangedEvent(true);
childChannelsCommand.store();

long baseChannelId = baseChannel.map(Channel::getId).orElse(-1L);
List<Long> childChannelIds = childChannels.stream().map(Channel::getId).collect(Collectors.toList());

if (baseChannel.isPresent() || childChannels.isEmpty()) {
UpdateBaseChannelCommand baseChannelCommand = new UpdateBaseChannelCommand(
user,
server,
baseChannelId);

baseChannelCommand.skipChannelChangedEvent(true);
baseChannelCommand.store();
}

if (!childChannels.isEmpty()) {
UpdateChildChannelsCommand childChannelsCommand = new UpdateChildChannelsCommand(
user,
server,
childChannelIds);

childChannelsCommand.skipChannelChangedEvent(true);
childChannelsCommand.store();
}

// Calling this asynchronous block execution util main thread close the Hibernate Session
// as we require the result, we must call this synchronous
Expand Down
21 changes: 19 additions & 2 deletions java/code/src/com/redhat/rhn/manager/system/test/SystemManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import com.redhat.rhn.domain.action.test.ActionFactoryTest;
import com.redhat.rhn.domain.channel.Channel;
import com.redhat.rhn.domain.channel.ChannelArch;
import com.redhat.rhn.domain.channel.ProductName;
import com.redhat.rhn.domain.channel.test.ChannelFactoryTest;
import com.redhat.rhn.domain.dto.SystemGroupID;
import com.redhat.rhn.domain.dto.SystemGroupsDTO;
Expand Down Expand Up @@ -104,7 +105,9 @@
import com.redhat.rhn.frontend.dto.SystemOverview;
import com.redhat.rhn.frontend.dto.VirtualSystemOverview;
import com.redhat.rhn.frontend.listview.PageControl;
import com.redhat.rhn.frontend.xmlrpc.ChannelSubscriptionException;
import com.redhat.rhn.manager.action.ActionManager;
import com.redhat.rhn.manager.content.MgrSyncUtils;
import com.redhat.rhn.manager.entitlement.EntitlementManager;
import com.redhat.rhn.manager.errata.cache.ErrataCacheManager;
import com.redhat.rhn.manager.formula.FormulaMonitoringManager;
Expand Down Expand Up @@ -157,6 +160,7 @@
import org.jmock.Expectations;
import org.jmock.imposters.ByteBuddyClassImposteriser;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.yaml.snakeyaml.Yaml;
Expand Down Expand Up @@ -1367,17 +1371,26 @@ public void testUpdateServerChannelsNoChildren() throws Exception {
this.getClass().getSimpleName());
Server server = ServerFactoryTest.createTestServer(user, true);

ProductName pnbase = MgrSyncUtils.findOrCreateProductName("Product Name Base");
ProductName pnch1 = MgrSyncUtils.findOrCreateProductName("Product Name Child 1");
ProductName pnch2 = MgrSyncUtils.findOrCreateProductName("Product Name Child 2");

Channel base1 = ChannelFactoryTest.createBaseChannel(user);
base1.setProductName(pnbase);
Channel ch11 = ChannelFactoryTest.createTestChannel(user.getOrg());
ch11.setProductName(pnch1);

ch11.setParentChannel(base1);

server.addChannel(base1);
server.addChannel(ch11);

Channel base2 = ChannelFactoryTest.createBaseChannel(user);
base2.setProductName(pnbase);
Channel ch21 = ChannelFactoryTest.createTestChannel(user.getOrg());
ch21.setProductName(pnch1);
Channel ch22 = ChannelFactoryTest.createTestChannel(user.getOrg());
ch22.setProductName(pnch2);
ch21.setParentChannel(base2);
ch22.setParentChannel(base2);

Expand All @@ -1386,7 +1399,8 @@ public void testUpdateServerChannelsNoChildren() throws Exception {
systemManager.updateServerChannels(user, server, of(base2), Collections.emptyList());

assertEquals(base2.getId(), server.getBaseChannel().getId());
assertEquals(0, server.getChildChannels().size());
assertEquals(1, server.getChildChannels().size());
assertEquals(ch21, server.getChildChannels().iterator().next());
}

@Test
Expand All @@ -1411,7 +1425,10 @@ public void testUpdateServerChannelsNoBase() throws Exception {

HibernateFactory.getSession().flush();

systemManager.updateServerChannels(user, server, empty(), Arrays.asList(ch21, ch22));
Assertions.assertThrows(ChannelSubscriptionException.class,
() -> systemManager.updateServerChannels(user, server, empty(), Arrays.asList(ch21, ch22)));

systemManager.updateServerChannels(user, server, empty(), Collections.emptyList());

assertNull(server.getBaseChannel());
assertEquals(0, server.getChildChannels().size());
Expand Down
33 changes: 30 additions & 3 deletions java/code/src/com/suse/manager/webui/services/SaltServerActionService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,26 @@ public void execute(Action actionIn, boolean forcePackageListRefresh,

private void executeForRegularMinions(Action actionIn, boolean forcePackageListRefresh,
boolean isStagingJob, Optional<Long> stagingJobMinionServerId, List<MinionSummary> minionSummaries) {
for (Map.Entry<LocalCall<?>, List<MinionSummary>> entry : callsForAction(actionIn, minionSummaries)
.entrySet()) {
Map<LocalCall<?>, List<MinionSummary>> localCalls = new HashMap<>();
try {
localCalls = callsForAction(actionIn, minionSummaries);
}
catch (RuntimeException e) {
LOG.error("Failed to prepare salt calls: ", e);
List<Long> failedServerIds = minionSummaries.stream()
.map(MinionSummary::getServerId).toList();
if (!failedServerIds.isEmpty()) {
actionIn.getServerActions().stream()
.filter(sa -> failedServerIds.contains(sa.getServer().getId()))
.forEach(sa -> {
sa.setStatus(STATUS_FAILED);
sa.setResultMsg("Error preparing salt call: " + e.getMessage());
sa.setCompletionTime(new Date());
});
}
return;
}
for (Map.Entry<LocalCall<?>, List<MinionSummary>> entry : localCalls.entrySet()) {
LocalCall<?> call = entry.getKey();
final List<MinionSummary> targetMinions;
Map<Boolean, List<MinionSummary>> results;
Expand Down Expand Up @@ -2481,7 +2499,16 @@ public void executeSSHAction(Action action, MinionServer minion, boolean forcePk

sa.setRemainingTries(sa.getRemainingTries() - 1);

Map<LocalCall<?>, List<MinionSummary>> calls = callsForAction(action, List.of(new MinionSummary(minion)));
Map<LocalCall<?>, List<MinionSummary>> calls = new HashMap<>();
try {
calls = callsForAction(action, List.of(new MinionSummary(minion)));
}
catch (RuntimeException e) {
sa.setStatus(STATUS_FAILED);
sa.setResultMsg("Error preparing salt call: " + e.getMessage());
sa.setCompletionTime(new Date());
return;
}

for (LocalCall<?> call : calls.keySet()) {
Optional<Result<JsonElement>> result;
Expand Down
4 changes: 4 additions & 0 deletions java/spacewalk-java.changes.mcalmer.improve-channel-change
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Support removing all channels with scheduleChangeChannels()
- Support finding compatible child channels when changing the
base channels with scheduleChangeChannels()
- Check consistence of base and child channels (bsc#1232713)

0 comments on commit b3cf9ef

Please sign in to comment.