Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.log4j.Logger;

import com.cloud.upgrade.dao.DbUpgrade;
import com.cloud.upgrade.dao.DbUpgradeSystemVmTemplate;
import com.cloud.upgrade.dao.Upgrade217to218;
import com.cloud.upgrade.dao.Upgrade218to22;
import com.cloud.upgrade.dao.Upgrade218to224DomainVlans;
Expand Down Expand Up @@ -232,11 +233,42 @@ DbUpgrade[] calculateUpgradePath(final CloudStackVersion dbVersion, final CloudS

}

private void updateSystemVmTemplates(DbUpgrade[] upgrades) {
for (int i = upgrades.length - 1; i >= 0; i--) {
DbUpgrade upgrade = upgrades[i];
if (upgrade instanceof DbUpgradeSystemVmTemplate) {
TransactionLegacy txn = TransactionLegacy.open("Upgrade");
txn.start();
try {
Connection conn;
try {
conn = txn.getConnection();
} catch (SQLException e) {
String errorMessage = "Unable to upgrade the database";
s_logger.error(errorMessage, e);
throw new CloudRuntimeException(errorMessage, e);
}
((DbUpgradeSystemVmTemplate)upgrade).updateSystemVmTemplates(conn);
txn.commit();
break;
} catch (CloudRuntimeException e) {
String errorMessage = "Unable to upgrade the database";
s_logger.error(errorMessage, e);
throw new CloudRuntimeException(errorMessage, e);
} finally {
txn.close();
Copy link
Contributor

@Pearl1594 Pearl1594 Feb 1, 2021

Choose a reason for hiding this comment

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

@ustcweizhou I believe there's a need to do a txn.commit() before closing the transaction as the DB changes made as part of the updateSystemVmTemplate() gets rolled back

Rolling back the transaction: Time = 22 Name =  Upgrade; called by -TransactionLegacy.rollback:888-TransactionLegacy.removeUpTo:831-TransactionLegacy.close:655-DatabaseUpgradeChecker.updateSystemVmTemplates:264-DatabaseUpgradeChecker.upgrade:275-DatabaseUpgradeChecker.check:379-CloudStackExtendedLifeCycle.checkIntegrity:64-CloudStackExtendedLifeCycle.start:54-DefaultLifecycleProcessor.doStart:182-DefaultLifecycleProcessor.access$200:53-DefaultLifecycleProcessor$LifecycleGroup.start:360-DefaultLifecycleProcessor.startBeans:158
2021-02-01 09:50:57,795 DEBUG [c.c.u.DatabaseUpgradeChecker] (main:null) (logid:) Running upgrade Upgrade41500to41510 to upgrade from 4.15.0.0-4.15.1.0 to 4.15.1.0

Copy link
Member

Choose a reason for hiding this comment

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

@ustcweizhou I believe there's a need to do a txn.commit() before closing the transaction as the DB changes made as part of the updateSystemVmTemplate() gets rolled back

Rolling back the transaction: Time = 22 Name =  Upgrade; called by -TransactionLegacy.rollback:888-TransactionLegacy.removeUpTo:831-TransactionLegacy.close:655-DatabaseUpgradeChecker.updateSystemVmTemplates:264-DatabaseUpgradeChecker.upgrade:275-DatabaseUpgradeChecker.check:379-CloudStackExtendedLifeCycle.checkIntegrity:64-CloudStackExtendedLifeCycle.start:54-DefaultLifecycleProcessor.doStart:182-DefaultLifecycleProcessor.access$200:53-DefaultLifecycleProcessor$LifecycleGroup.start:360-DefaultLifecycleProcessor.startBeans:158
2021-02-01 09:50:57,795 DEBUG [c.c.u.DatabaseUpgradeChecker] (main:null) (logid:) Running upgrade Upgrade41500to41510 to upgrade from 4.15.0.0-4.15.1.0 to 4.15.1.0

@Pearl1594 thanks for testing.
could you tell how to reproduce the issue ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@weizhouapache I tested this by merging this code to master. However, the steps are equivalent to :

  1. Deploy a 4.14 env
  2. Register the template (4.15 template)
  3. upgrade to 4.15
  4. The Management server comes up fine, but, if we check the cloud.configuration value (in the DB) for attribute - "router.template." notice it doesn't update the template name to the new one.

The case of not applying the DB changes when template isn't registered, works absolutely fine.

Copy link
Member

Choose a reason for hiding this comment

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

@Pearl1594 yeah, you are right !
pushed a change.
thanks for review and testing.

}
}
}
}

protected void upgrade(CloudStackVersion dbVersion, CloudStackVersion currentVersion) {
s_logger.info("Database upgrade must be performed from " + dbVersion + " to " + currentVersion);

final DbUpgrade[] upgrades = calculateUpgradePath(dbVersion, currentVersion);

updateSystemVmTemplates(upgrades);

for (DbUpgrade upgrade : upgrades) {
VersionVO version;
s_logger.debug("Running upgrade " + upgrade.getClass().getSimpleName() + " to upgrade from " + upgrade.getUpgradableVersionRange()[0] + "-" + upgrade
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// 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 com.cloud.upgrade.dao;

import java.sql.Connection;

public interface DbUpgradeSystemVmTemplate {

void updateSystemVmTemplates(Connection conn);
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import com.cloud.hypervisor.Hypervisor;
import com.cloud.utils.exception.CloudRuntimeException;

public class Upgrade41400to41500 implements DbUpgrade {
public class Upgrade41400to41500 implements DbUpgrade, DbUpgradeSystemVmTemplate {

final static Logger LOG = Logger.getLogger(Upgrade41400to41500.class);

Expand Down Expand Up @@ -67,12 +67,12 @@ public InputStream[] getPrepareScripts() {

@Override
public void performDataMigration(Connection conn) {
updateSystemVmTemplates(conn);
Copy link
Member

Choose a reason for hiding this comment

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

+1 important thing to remember now is not to run this again

addRolePermissionsForNewReadOnlyAndSupportRoles(conn);
}

@Override
@SuppressWarnings("serial")
private void updateSystemVmTemplates(final Connection conn) {
public void updateSystemVmTemplates(final Connection conn) {
LOG.debug("Updating System Vm template IDs");
final Set<Hypervisor.HypervisorType> hypervisorsListInUse = new HashSet<Hypervisor.HypervisorType>();
try (PreparedStatement pstmt = conn.prepareStatement("select distinct(hypervisor_type) from `cloud`.`cluster` where removed is null"); ResultSet rs = pstmt.executeQuery()) {
Expand Down