[pve-devel] [PATCH ha-manager 1/2] rename request state 'enabled' to 'started'

Thomas Lamprecht t.lamprecht at proxmox.com
Fri Nov 18 16:53:01 CET 2016


with the new 'stopped' state there was a possible confusion what each
state could do. Rename enabled to started to started to address this.

Enabled can still be used for backward compatibility and will be
removed on the next major release.

The TestHardware does not recognizes the "service enable" command
anymore, the regression test which made use of it were ported to
"service started" command.

Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---

I did not split the regression test syntax changes out in an own
patch as I would need to add code now for backwards compatibility
(map service enabled to service started) which then would be removed
in the next patch. Also the changes are clearly non-behavioral so I
did not bother.

But if it eases review I can split them up

 src/PVE/HA/Config.pm                                   |  5 +++--
 src/PVE/HA/Manager.pm                                  | 12 ++++++------
 src/PVE/HA/Resources.pm                                |  2 +-
 src/PVE/HA/Sim/Hardware.pm                             |  1 +
 src/PVE/HA/Sim/RTHardware.pm                           |  4 ++--
 src/PVE/HA/Sim/TestHardware.pm                         |  6 +++---
 src/PVE/HA/Tools.pm                                    |  8 ++++----
 src/test/test-relocate-policy-default-group/cmdlist    |  2 +-
 src/test/test-relocate-policy-default-group/log.expect |  2 +-
 src/test/test-relocate-policy1/cmdlist                 |  2 +-
 src/test/test-relocate-policy1/log.expect              |  2 +-
 src/test/test-resource-failure1/cmdlist                |  2 +-
 src/test/test-resource-failure1/log.expect             |  2 +-
 src/test/test-resource-failure2/cmdlist                |  2 +-
 src/test/test-resource-failure2/log.expect             |  2 +-
 src/test/test-resource-failure5/cmdlist                |  2 +-
 src/test/test-resource-failure5/log.expect             |  2 +-
 src/test/test-resource-failure6/cmdlist                |  2 +-
 src/test/test-resource-failure6/log.expect             |  2 +-
 src/test/test-service-command1/cmdlist                 |  2 +-
 src/test/test-service-command1/log.expect              |  2 +-
 21 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index 1802a7d..61cb70c 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -95,7 +95,8 @@ sub read_and_check_resources_config {
     foreach my $sid (keys %{$res->{ids}}) {
 	my $d = $res->{ids}->{$sid};
 	my (undef, undef, $name) = PVE::HA::Tools::parse_sid($sid);
-	$d->{state} = 'enabled' if !defined($d->{state});
+	$d->{state} = 'started' if !defined($d->{state});
+	$d->{state} = 'started' if $d->{state} eq 'enabled'; # backward compatibility
 	$d->{max_restart} = 1 if !defined($d->{max_restart});
 	$d->{max_relocate} = 1 if !defined($d->{max_relocate});
 	if (PVE::HA::Resources->lookup($d->{type})) {
@@ -202,7 +203,7 @@ my $service_check_ha_state = sub {
     if (my $d = $conf->{ids}->{$sid}) {
 	return 1 if !defined($has_state);
 
-	$d->{state} = 'enabled' if !defined($d->{state});
+	$d->{state} = 'started' if !defined($d->{state});
 	return 1 if $d->{state} eq $has_state;
     }
 
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 9d333fb..8446b29 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -290,7 +290,7 @@ my $recover_fenced_service = sub {
 
 	# $sd *is normally read-only*, fencing is the exception
 	$cd->{node} = $sd->{node} = $recovery_node;
-	my $new_state = ($cd->{state} eq 'enabled') ? 'started' : 'request_stop';
+	my $new_state = ($cd->{state} eq 'started') ? 'started' : 'request_stop';
 	&$change_service_state($self, $sid, $new_state, node => $recovery_node);
     } else {
 	# no possible node found, cannot recover
@@ -383,7 +383,7 @@ sub manage {
 	my $cd = $sc->{$sid};
 	$haenv->log('info', "adding new service '$sid' on node '$cd->{node}'");
 	# assume we are running to avoid relocate running service at add
-	my $state = ($cd->{state} eq 'enabled') ? 'started' : 'request_stop';
+	my $state = ($cd->{state} eq 'started') ? 'started' : 'request_stop';
 	$ss->{$sid} = { state => $state, node => $cd->{node},
 			uid => compute_new_uuid('started') };
     }
@@ -435,7 +435,7 @@ sub manage {
 
 		my $lrm_mode = $sd->{node} ? $lrm_modes->{$sd->{node}} : undef;
 		# unfreeze
-		my $state = ($cd->{state} eq 'enabled') ? 'started' : 'request_stop';
+		my $state = ($cd->{state} eq 'started') ? 'started' : 'request_stop';
 		&$change_service_state($self, $sid, $state)
 		    if $lrm_mode && $lrm_mode eq 'active';
 
@@ -522,7 +522,7 @@ sub next_state_migrate_relocate {
     # check result from LRM daemon
     if ($lrm_res) {
 	my $exit_code = $lrm_res->{exit_code};
-	my $req_state = $cd->{state} eq 'enabled' ? 'started' : 'request_stop';
+	my $req_state = $cd->{state} eq 'started' ? 'started' : 'request_stop';
 	if ($exit_code == SUCCESS) {
 	    &$change_service_state($self, $sid, $req_state, node => $sd->{target});
 	    return;
@@ -593,7 +593,7 @@ sub next_state_stopped {
 	return;
     }
 
-    if ($cd->{state} eq 'enabled') {
+    if ($cd->{state} eq 'started') {
 	# simply mark it started, if it's on the wrong node
 	# next_state_started will fix that for us
 	&$change_service_state($self, $sid, 'started', node => $sd->{node});
@@ -632,7 +632,7 @@ sub next_state_started {
 	return;
     }
 
-    if ($cd->{state} eq 'enabled') {
+    if ($cd->{state} eq 'started') {
 
 	if ($sd->{cmd}) {
 	    my ($cmd, $target) = @{$sd->{cmd}};
diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm
index 5460072..b1c98e6 100644
--- a/src/PVE/HA/Resources.pm
+++ b/src/PVE/HA/Resources.pm
@@ -19,7 +19,7 @@ my $defaultData = {
 	    type => 'string',
 	    enum => ['started', 'stopped', 'enabled', 'disabled'],
 	    optional => 1,
-	    default => 'enabled',
+	    default => 'started',
 	    description => <<EODESC,
 Requested resource state. The CRM reads this state and acts accordingly.
 Please note that `enabled` is just an alias for `started`.
diff --git a/src/PVE/HA/Sim/Hardware.pm b/src/PVE/HA/Sim/Hardware.pm
index 383b10e..96186de 100644
--- a/src/PVE/HA/Sim/Hardware.pm
+++ b/src/PVE/HA/Sim/Hardware.pm
@@ -101,6 +101,7 @@ sub read_service_config {
 	    die "implement me";
 	}
 	$d->{state} = 'disabled' if !$d->{state};
+	$d->{state} = 'started' if $d->{state} eq 'enabled'; # backward compatibility
 	$d->{max_restart} = 1 if !defined($d->{max_restart});
 	$d->{max_relocate} = 1 if !defined($d->{max_relocate});
     }
diff --git a/src/PVE/HA/Sim/RTHardware.pm b/src/PVE/HA/Sim/RTHardware.pm
index 866d0bc..606d3ef 100644
--- a/src/PVE/HA/Sim/RTHardware.pm
+++ b/src/PVE/HA/Sim/RTHardware.pm
@@ -316,7 +316,7 @@ sub set_service_state {
     my ($self, $sid) = @_;
 
     my $d = $self->{service_gui}->{$sid} || die "no such service '$sid'";
-    my $state = $d->{enable_btn}->get_active() ? 'enabled' : 'disabled';
+    my $state = $d->{enable_btn}->get_active() ? 'started' : 'disabled';
 
     $self->{service_config} = $self->SUPER::set_service_state($sid, $state);
 
@@ -461,7 +461,7 @@ sub create_service_control {
 
 	$w = Gtk3::Switch->new();
 	$sgrid->attach($w, 1, $row, 1, 1);
-	$w->set_active(1) if $d->{state} eq 'enabled';
+	$w->set_active(1) if $d->{state} eq 'started';
 	$self->{service_gui}->{$sid}->{enable_btn} = $w;
 	$w->signal_connect('notify::active' => sub {
 	    $self->set_service_state($sid);
diff --git a/src/PVE/HA/Sim/TestHardware.pm b/src/PVE/HA/Sim/TestHardware.pm
index c6ad238..50aef0c 100644
--- a/src/PVE/HA/Sim/TestHardware.pm
+++ b/src/PVE/HA/Sim/TestHardware.pm
@@ -87,7 +87,7 @@ sub log {
 # reboot <node>
 # shutdown <node>
 # restart-lrm <node>
-# service <sid> <enabled|disabled|stopped>
+# service <sid> <started|disabled|stopped>
 # service <sid> <migrate|relocate> <target>
 # service <sid> lock/unlock [lockname]
 
@@ -175,7 +175,7 @@ sub sim_hardware_cmd {
 	    }
 
 	} elsif ($cmd eq 'service') {
-	    if ($action eq 'enabled' || $action eq 'disabled' || $action eq 'stopped') {
+	    if ($action eq 'started' || $action eq 'disabled' || $action eq 'stopped') {
 
 		$self->set_service_state($sid, $action);
 
@@ -188,7 +188,7 @@ sub sim_hardware_cmd {
 
 	    } elsif ($action eq 'add') {
 
-		$self->add_service($sid, {state => 'enabled', node => $target});
+		$self->add_service($sid, {state => 'started', node => $target});
 
 	    } elsif ($action eq 'delete') {
 
diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm
index 072ef8f..a4289f5 100644
--- a/src/PVE/HA/Tools.pm
+++ b/src/PVE/HA/Tools.pm
@@ -239,8 +239,8 @@ sub complete_enabled_sid {
 
     my $res = [];
     foreach my $sid (keys %{$cfg->{ids}}) {
-	my $state = $cfg->{ids}->{$sid}->{state} // 'enabled';
-	next if $state ne 'enabled';
+	my $state = $cfg->{ids}->{$sid}->{state} // 'started';
+	next if $state ne 'started';
 	push @$res, $sid;
     }
 
@@ -253,8 +253,8 @@ sub complete_disabled_sid {
 
     my $res = [];
     foreach my $sid (keys %{$cfg->{ids}}) {
-	my $state = $cfg->{ids}->{$sid}->{state} // 'enabled';
-	next if $state eq 'enabled';
+	my $state = $cfg->{ids}->{$sid}->{state} // 'started';
+	next if $state eq 'started';
 	push @$res, $sid;
     }
 
diff --git a/src/test/test-relocate-policy-default-group/cmdlist b/src/test/test-relocate-policy-default-group/cmdlist
index 8f06508..0d0a11d 100644
--- a/src/test/test-relocate-policy-default-group/cmdlist
+++ b/src/test/test-relocate-policy-default-group/cmdlist
@@ -1,4 +1,4 @@
 [
     [ "power node1 on", "power node2 on", "power node3 on"],
-    [ "service fa:130 enabled" ]
+    [ "service fa:130 started" ]
 ]
diff --git a/src/test/test-relocate-policy-default-group/log.expect b/src/test/test-relocate-policy-default-group/log.expect
index 694bef6..e0ae1ab 100644
--- a/src/test/test-relocate-policy-default-group/log.expect
+++ b/src/test/test-relocate-policy-default-group/log.expect
@@ -19,7 +19,7 @@ info     23    node2/lrm: got lock 'ha_agent_node2_lock'
 info     23    node2/lrm: status change wait_for_agent_lock => active
 info     24    node3/crm: status change wait_for_quorum => slave
 info     40    node1/crm: service 'fa:130': state changed from 'request_stop' to 'stopped'
-info    120      cmdlist: execute service fa:130 enabled
+info    120      cmdlist: execute service fa:130 started
 info    120    node1/crm: service 'fa:130': state changed from 'stopped' to 'started'  (node = node2)
 info    123    node2/lrm: starting service fa:130
 warn    123    node2/lrm: unable to start service fa:130
diff --git a/src/test/test-relocate-policy1/cmdlist b/src/test/test-relocate-policy1/cmdlist
index d253427..749bffe 100644
--- a/src/test/test-relocate-policy1/cmdlist
+++ b/src/test/test-relocate-policy1/cmdlist
@@ -1,4 +1,4 @@
 [
     [ "power node1 on", "power node2 on", "power node3 on" ],
-    [ "service fa:130 enabled" ]
+    [ "service fa:130 started" ]
 ]
diff --git a/src/test/test-relocate-policy1/log.expect b/src/test/test-relocate-policy1/log.expect
index 834284b..e1c9f01 100644
--- a/src/test/test-relocate-policy1/log.expect
+++ b/src/test/test-relocate-policy1/log.expect
@@ -32,7 +32,7 @@ info     25    node3/lrm: status change wait_for_agent_lock => active
 info     25    node3/lrm: starting service vm:105
 info     25    node3/lrm: service status vm:105 started
 info     40    node1/crm: service 'fa:130': state changed from 'request_stop' to 'stopped'
-info    120      cmdlist: execute service fa:130 enabled
+info    120      cmdlist: execute service fa:130 started
 info    120    node1/crm: service 'fa:130': state changed from 'stopped' to 'started'  (node = node3)
 info    125    node3/lrm: starting service fa:130
 warn    125    node3/lrm: unable to start service fa:130
diff --git a/src/test/test-resource-failure1/cmdlist b/src/test/test-resource-failure1/cmdlist
index 5489743..be41e90 100644
--- a/src/test/test-resource-failure1/cmdlist
+++ b/src/test/test-resource-failure1/cmdlist
@@ -1,4 +1,4 @@
 [
     [ "power node1 on", "power node2 on", "power node3 on"],
-    [ "service fa:110 enabled" ]
+    [ "service fa:110 started" ]
 ]
diff --git a/src/test/test-resource-failure1/log.expect b/src/test/test-resource-failure1/log.expect
index 8439778..129d66e 100644
--- a/src/test/test-resource-failure1/log.expect
+++ b/src/test/test-resource-failure1/log.expect
@@ -19,7 +19,7 @@ info     23    node2/lrm: got lock 'ha_agent_node2_lock'
 info     23    node2/lrm: status change wait_for_agent_lock => active
 info     24    node3/crm: status change wait_for_quorum => slave
 info     40    node1/crm: service 'fa:110': state changed from 'request_stop' to 'stopped'
-info    120      cmdlist: execute service fa:110 enabled
+info    120      cmdlist: execute service fa:110 started
 info    120    node1/crm: service 'fa:110': state changed from 'stopped' to 'started'  (node = node2)
 info    123    node2/lrm: starting service fa:110
 warn    123    node2/lrm: unable to start service fa:110
diff --git a/src/test/test-resource-failure2/cmdlist b/src/test/test-resource-failure2/cmdlist
index 8f06508..0d0a11d 100644
--- a/src/test/test-resource-failure2/cmdlist
+++ b/src/test/test-resource-failure2/cmdlist
@@ -1,4 +1,4 @@
 [
     [ "power node1 on", "power node2 on", "power node3 on"],
-    [ "service fa:130 enabled" ]
+    [ "service fa:130 started" ]
 ]
diff --git a/src/test/test-resource-failure2/log.expect b/src/test/test-resource-failure2/log.expect
index 66ddc04..b672dbb 100644
--- a/src/test/test-resource-failure2/log.expect
+++ b/src/test/test-resource-failure2/log.expect
@@ -19,7 +19,7 @@ info     23    node2/lrm: got lock 'ha_agent_node2_lock'
 info     23    node2/lrm: status change wait_for_agent_lock => active
 info     24    node3/crm: status change wait_for_quorum => slave
 info     40    node1/crm: service 'fa:130': state changed from 'request_stop' to 'stopped'
-info    120      cmdlist: execute service fa:130 enabled
+info    120      cmdlist: execute service fa:130 started
 info    120    node1/crm: service 'fa:130': state changed from 'stopped' to 'started'  (node = node2)
 info    123    node2/lrm: starting service fa:130
 warn    123    node2/lrm: unable to start service fa:130
diff --git a/src/test/test-resource-failure5/cmdlist b/src/test/test-resource-failure5/cmdlist
index ff47c70..109b215 100644
--- a/src/test/test-resource-failure5/cmdlist
+++ b/src/test/test-resource-failure5/cmdlist
@@ -1,5 +1,5 @@
 [
     [ "power node1 on", "power node2 on", "power node3 on"],
-    [ "service fa:130 enabled" ],
+    [ "service fa:130 started" ],
     [ "service fa:130 disabled" ]
 ]
diff --git a/src/test/test-resource-failure5/log.expect b/src/test/test-resource-failure5/log.expect
index 4396691..2fbd5ef 100644
--- a/src/test/test-resource-failure5/log.expect
+++ b/src/test/test-resource-failure5/log.expect
@@ -19,7 +19,7 @@ info     23    node2/lrm: got lock 'ha_agent_node2_lock'
 info     23    node2/lrm: status change wait_for_agent_lock => active
 info     24    node3/crm: status change wait_for_quorum => slave
 info     40    node1/crm: service 'fa:130': state changed from 'request_stop' to 'stopped'
-info    120      cmdlist: execute service fa:130 enabled
+info    120      cmdlist: execute service fa:130 started
 info    120    node1/crm: service 'fa:130': state changed from 'stopped' to 'started'  (node = node2)
 info    123    node2/lrm: starting service fa:130
 warn    123    node2/lrm: unable to start service fa:130
diff --git a/src/test/test-resource-failure6/cmdlist b/src/test/test-resource-failure6/cmdlist
index 8f06508..0d0a11d 100644
--- a/src/test/test-resource-failure6/cmdlist
+++ b/src/test/test-resource-failure6/cmdlist
@@ -1,4 +1,4 @@
 [
     [ "power node1 on", "power node2 on", "power node3 on"],
-    [ "service fa:130 enabled" ]
+    [ "service fa:130 started" ]
 ]
diff --git a/src/test/test-resource-failure6/log.expect b/src/test/test-resource-failure6/log.expect
index 5738b82..61992cb 100644
--- a/src/test/test-resource-failure6/log.expect
+++ b/src/test/test-resource-failure6/log.expect
@@ -19,7 +19,7 @@ info     23    node2/lrm: got lock 'ha_agent_node2_lock'
 info     23    node2/lrm: status change wait_for_agent_lock => active
 info     24    node3/crm: status change wait_for_quorum => slave
 info     40    node1/crm: service 'fa:130': state changed from 'request_stop' to 'stopped'
-info    120      cmdlist: execute service fa:130 enabled
+info    120      cmdlist: execute service fa:130 started
 info    120    node1/crm: service 'fa:130': state changed from 'stopped' to 'started'  (node = node2)
 info    123    node2/lrm: starting service fa:130
 warn    123    node2/lrm: unable to start service fa:130
diff --git a/src/test/test-service-command1/cmdlist b/src/test/test-service-command1/cmdlist
index 815901d..ca0af75 100644
--- a/src/test/test-service-command1/cmdlist
+++ b/src/test/test-service-command1/cmdlist
@@ -1,5 +1,5 @@
 [
     [ "power node1 on", "power node2 on", "power node3 on"],
     [ "service vm:103 disabled" ],
-    [ "service vm:103 enabled" ]
+    [ "service vm:103 started" ]
 ]
diff --git a/src/test/test-service-command1/log.expect b/src/test/test-service-command1/log.expect
index 7e74e8a..02ca900 100644
--- a/src/test/test-service-command1/log.expect
+++ b/src/test/test-service-command1/log.expect
@@ -25,7 +25,7 @@ info    120    node1/crm: service 'vm:103': state changed from 'started' to 'req
 info    125    node3/lrm: stopping service vm:103
 info    125    node3/lrm: service status vm:103 stopped
 info    140    node1/crm: service 'vm:103': state changed from 'request_stop' to 'stopped'
-info    220      cmdlist: execute service vm:103 enabled
+info    220      cmdlist: execute service vm:103 started
 info    220    node1/crm: service 'vm:103': state changed from 'stopped' to 'started'  (node = node3)
 info    225    node3/lrm: starting service vm:103
 info    225    node3/lrm: service status vm:103 started
-- 
2.1.4





More information about the pve-devel mailing list