Fix get_status api in COS policy manager
GetStatus was deprecated in Update Engine. This broke the get_status api
in COS Policy Manager. Changing the implementation to use
GetStatusAdvanced api instead.
BUG=b/219970525
TEST=presubmit and manually tested on VM
RELEASE_NOTES=Fix get_status api in COS Policy Manager
Change-Id: Ia0d0db2f5552cf528fb0863da1011aa1365a5a34
diff --git a/src/policy_manager/dbus/update_engine_client.go b/src/policy_manager/dbus/update_engine_client.go
index 5b84086..bc0da24 100644
--- a/src/policy_manager/dbus/update_engine_client.go
+++ b/src/policy_manager/dbus/update_engine_client.go
@@ -14,6 +14,8 @@
package dbus
+import "policy_manager/policymanagerproto"
+
// UpdateEngineClient is the generic interface for interacting with update-engine.
type UpdateEngineClient interface {
// SetChannel Sets the target channel for update.
@@ -23,7 +25,7 @@
AttemptUpdate() error
// GetStatus returns the update status.
- GetStatus() (response UEGetStatusResponse, err error)
+ GetStatus() (*policymanagerproto.StatusResult, error)
// SubscribeStatusUpdate subscribes the status update of update-engine.
// It returns a channel so that any udpate status change of udpate-engine
diff --git a/src/policy_manager/dbus/update_engine_client_impl.go b/src/policy_manager/dbus/update_engine_client_impl.go
index 420a3aa..35e6742 100644
--- a/src/policy_manager/dbus/update_engine_client_impl.go
+++ b/src/policy_manager/dbus/update_engine_client_impl.go
@@ -16,21 +16,23 @@
import (
"fmt"
+ "policy_manager/policymanagerproto"
"strings"
godbus "github.com/godbus/dbus"
+ "github.com/golang/protobuf/proto"
)
// Implementation of the UpdateEngineClient interface that interacts with update-engine.
const (
- updateEngineObjectName = "org.chromium.UpdateEngine"
- updateEngineObjectPath = "/org/chromium/UpdateEngine"
- setChannelMethod = "org.chromium.UpdateEngineInterface.SetChannel"
- attemptUpdateMethod = "org.chromium.UpdateEngineInterface.AttemptUpdate"
- getStatusMethod = "org.chromium.UpdateEngineInterface.GetStatus"
- statusUpdateSignal = "org.chromium.UpdateEngineInterface.StatusUpdate"
- dBusAddMatchMethod = "org.freedesktop.DBus.AddMatch"
+ updateEngineObjectName = "org.chromium.UpdateEngine"
+ updateEngineObjectPath = "/org/chromium/UpdateEngine"
+ setChannelMethod = "org.chromium.UpdateEngineInterface.SetChannel"
+ attemptUpdateMethod = "org.chromium.UpdateEngineInterface.AttemptUpdate"
+ getStatusAdvancedMethod = "org.chromium.UpdateEngineInterface.GetStatusAdvanced"
+ statusUpdateSignal = "org.chromium.UpdateEngineInterface.StatusUpdate"
+ dBusAddMatchMethod = "org.freedesktop.DBus.AddMatch"
)
type updateEngineClientImpl struct {
@@ -72,15 +74,18 @@
// GetStatus returns the update status
func (client *updateEngineClientImpl) GetStatus() (
- response UEGetStatusResponse, err error) {
- err = client.conn.Object(updateEngineObjectName, updateEngineObjectPath).Call(
- getStatusMethod, 0).Store(
- &response.LastCheckedTime,
- &response.Progress,
- &response.UpdateStatus,
- &response.NewVersion,
- &response.NewSize)
- return
+ *policymanagerproto.StatusResult, error) {
+ var updateStatusBytes []byte
+ updateStatus := new(policymanagerproto.StatusResult)
+ if err := client.conn.Object(updateEngineObjectName, updateEngineObjectPath).Call(
+ getStatusAdvancedMethod, 0).Store(&updateStatusBytes); err != nil {
+ return nil, fmt.Errorf("failed to fetch update status: %s", err)
+ }
+ if err := proto.Unmarshal(updateStatusBytes, updateStatus); err != nil {
+ return nil, fmt.Errorf("failed to unmarshal UpdateStatus %s: %s",
+ updateStatusBytes, updateStatus)
+ }
+ return updateStatus, nil
}
// SubscribeStatusUpdate subscribes the status update of update-engine.
diff --git a/src/policy_manager/dbus/update_engine_client_impl_test.go b/src/policy_manager/dbus/update_engine_client_impl_test.go
index 074fe46..0e8b3c5 100644
--- a/src/policy_manager/dbus/update_engine_client_impl_test.go
+++ b/src/policy_manager/dbus/update_engine_client_impl_test.go
@@ -17,6 +17,8 @@
import (
"errors"
+ "github.com/golang/protobuf/proto"
+ "policy_manager/policymanagerproto"
"reflect"
"testing"
@@ -122,25 +124,28 @@
// TestGetStatus tests GetStatus returns correct values.
func TestGetStatus(t *testing.T) {
+ rebootStatus := policymanagerproto.Operation_UPDATED_NEED_REBOOT
tests := []struct {
name string
- expectedReturn UEGetStatusResponse
+ expectedReturn *policymanagerproto.StatusResult
expectError bool
}{
{
"NormalCase",
- UEGetStatusResponse{
- 1433971203,
- 0.000000,
- "UPDATE_STATUS_UPDATED_NEED_REBOOT",
- "7150.0.0",
- 66542575,
+ &policymanagerproto.StatusResult{
+ NewVersion: proto.String("16550.0.0"),
+ UpdateStatus: &rebootStatus,
+ LastCheckedTime: proto.Int64(1433971203),
},
false,
},
{
"ErrorCase",
- UEGetStatusResponse{},
+ &policymanagerproto.StatusResult{
+ NewVersion: proto.String("16550.0.0"),
+ UpdateStatus: nil,
+ LastCheckedTime: proto.Int64(1433971203),
+ },
true,
},
}
@@ -149,21 +154,21 @@
// Mock dbus connection and object.
fakeConn := NewFakeConn()
fakeObj := NewFakeBusObject()
+ updateStatusBytes, err := proto.Marshal(test.expectedReturn)
+ if err != nil {
+ t.Errorf("Failed to marshal UpdateStatusBytes %v", updateStatusBytes)
+ }
fakeCall := func(args ...interface{}) ([]interface{}, error) {
if !test.expectError {
return []interface{}{
- test.expectedReturn.LastCheckedTime,
- test.expectedReturn.Progress,
- test.expectedReturn.UpdateStatus,
- test.expectedReturn.NewVersion,
- test.expectedReturn.NewSize,
+ updateStatusBytes,
}, nil
} else {
return nil, errors.New("Expected error.")
}
}
fakeConn.SetObject(updateEngineObjectName, updateEngineObjectPath, fakeObj)
- fakeObj.SetCall(getStatusMethod, fakeCall)
+ fakeObj.SetCall(getStatusAdvancedMethod, fakeCall)
ueClient := newFakeUpdateEngineClient(fakeConn)
ret, err := ueClient.GetStatus()
@@ -171,7 +176,7 @@
if err == nil {
if test.expectError {
t.Errorf("Test %s didn't get expected error", test.name)
- } else if !reflect.DeepEqual(test.expectedReturn, ret) {
+ } else if !proto.Equal(test.expectedReturn, ret) {
t.Errorf("Test %s got unexpected return %v, want %v", test.name, ret, test.expectedReturn)
}
} else if !test.expectError {
diff --git a/src/policy_manager/imgstatus/reporter_impl.go b/src/policy_manager/imgstatus/reporter_impl.go
index db4104d..5bf09cf 100644
--- a/src/policy_manager/imgstatus/reporter_impl.go
+++ b/src/policy_manager/imgstatus/reporter_impl.go
@@ -134,7 +134,7 @@
// Parse lsb-release file as key value pairs.
pairs, err := parseKeyValuePairs(bytes.NewReader(content))
if err != nil {
- return nil, fmt.Errorf("Unable to parse '%s': %s", lsbFile, err)
+ return nil, fmt.Errorf("unable to parse '%s': %s", lsbFile, err)
}
osVersion := new(pmpb.OSVersion)
@@ -171,19 +171,18 @@
status := new(pmpb.InstanceStatus)
- // Set update status string.
- statVal, ok := pmpb.InstanceStatus_UpdateStatus_value[response.UpdateStatus]
- if ok {
- statEnumVal := pmpb.InstanceStatus_UpdateStatus(statVal)
- status.UpdateStatus = &statEnumVal
+ // Set update status.
+ status.UpdateStatus = response.UpdateStatus
+ if status.UpdateStatus == nil {
+ return nil, fmt.Errorf("failed to fetch update status: It is possible that update_engine has not fetched the status yet.")
}
// Set new OS version.
status.NewOsVersion = new(pmpb.OSVersion)
- status.NewOsVersion.VersionString = proto.String(response.NewVersion)
+ status.NewOsVersion.VersionString = proto.String(*response.NewVersion)
// Set last update check timestamp.
- status.UpdateCheckTimestamp = proto.Int64(response.LastCheckedTime)
+ status.UpdateCheckTimestamp = proto.Int64(*response.LastCheckedTime)
return status, nil
}
@@ -224,7 +223,7 @@
// Get update status from update-engine via dbus.
updateStatus, err := getStatusFromUpdateEngine(rep.ueClient)
if err != nil {
- status.UpdateStatus = pmpb.InstanceStatus_UPDATE_STATUS_UNKNOWN.Enum()
+ status.UpdateStatus = nil
status.Error = proto.String(err.Error())
return status, err
}
diff --git a/src/policy_manager/imgstatus/reporter_impl_test.go b/src/policy_manager/imgstatus/reporter_impl_test.go
index ea2f5c6..72839ce 100644
--- a/src/policy_manager/imgstatus/reporter_impl_test.go
+++ b/src/policy_manager/imgstatus/reporter_impl_test.go
@@ -252,21 +252,19 @@
// TestGetStatusFromUpdateEngine tests that we can correctly get the update
// status from update-engine via dbus.
func TestGetStatusFromUpdateEngine(t *testing.T) {
- rebootStatus := policymanagerproto.InstanceStatus_UPDATE_STATUS_UPDATED_NEED_REBOOT
+ rebootStatus := policymanagerproto.Operation_UPDATED_NEED_REBOOT
tests := []struct {
name string
- getStatusResponse dbus.UEGetStatusResponse
+ getStatusResponse *policymanagerproto.StatusResult
expectedRet *policymanagerproto.InstanceStatus
expectedError error
}{
{
"GoodUpdateEngineOutput",
- dbus.UEGetStatusResponse{
- int64(1433971203),
- 0.000000,
- "UPDATE_STATUS_UPDATED_NEED_REBOOT",
- "16550.0.0",
- int64(66542575),
+ &policymanagerproto.StatusResult{
+ NewVersion: proto.String("16550.0.0"),
+ UpdateStatus: &rebootStatus,
+ LastCheckedTime: proto.Int64(1433971203),
},
&policymanagerproto.InstanceStatus{
NewOsVersion: &policymanagerproto.OSVersion{
@@ -279,12 +277,10 @@
},
{
"BadUpdateEngineOutput",
- dbus.UEGetStatusResponse{
- 0,
- 0,
- "",
- "",
- 0,
+ &policymanagerproto.StatusResult{
+ NewVersion: proto.String(""),
+ UpdateStatus: nil,
+ LastCheckedTime: proto.Int64(0),
},
nil,
errors.New("GetStatus error"),
@@ -330,13 +326,13 @@
mockAPI.EXPECT().ReadFile("/etc/lsb-release").Return([]byte(testLSB), nil)
mockClient := mockdbus.NewMockUpdateEngineClient(mockCtrl)
+ rebootStatus := policymanagerproto.Operation_UPDATED_NEED_REBOOT
+
mockClient.EXPECT().GetStatus().Return(
- dbus.UEGetStatusResponse{
- int64(1433971203),
- 0.000000,
- "UPDATE_STATUS_UPDATED_NEED_REBOOT",
- "16550.0.0",
- int64(66542575),
+ &policymanagerproto.StatusResult{
+ NewVersion: proto.String("16550.0.0"),
+ UpdateStatus: &rebootStatus,
+ LastCheckedTime: proto.Int64(1433971203),
}, nil)
mockPolicyEnforcer := mockpolicyenforcer.NewMockPolicyEnforcer(mockCtrl)
@@ -359,13 +355,11 @@
VersionString: proto.String("16550.0.0"),
}
- updateStatus := policymanagerproto.InstanceStatus_UPDATE_STATUS_UPDATED_NEED_REBOOT
-
want := &policymanagerproto.InstanceStatus{
InstanceId: proto.Uint64(testInstanceID),
OsVersion: ¤tVersion,
NewOsVersion: &newVersion,
- UpdateStatus: &updateStatus,
+ UpdateStatus: &rebootStatus,
UpdateCheckTimestamp: proto.Int64(1433971203),
HealthMonitorStatus: &policymanagerproto.HealthMonitorStatus{
Logging: proto.Bool(true),
@@ -422,11 +416,16 @@
Return([]byte{}, test.lsbErr).AnyTimes()
mockClient := mockdbus.NewMockUpdateEngineClient(mockCtrl)
+ errorStatus := policymanagerproto.Operation_ERROR
+
// Will only call GetStatus() if we didn't get error from ReadFile().
if test.lsbErr == nil {
mockClient.EXPECT().GetStatus().Return(
- dbus.UEGetStatusResponse{
- int64(0), float64(0), "", "", int64(0)},
+ &policymanagerproto.StatusResult{
+ NewVersion: proto.String(""),
+ UpdateStatus: &errorStatus,
+ LastCheckedTime: proto.Int64(0),
+ },
test.updateEngineErr)
}
diff --git a/src/policy_manager/main.go b/src/policy_manager/main.go
index 85de4dd..362ea15 100644
--- a/src/policy_manager/main.go
+++ b/src/policy_manager/main.go
@@ -278,18 +278,18 @@
resolveEnforcementConfig(userConfig, manager)
if err = resolveLocalUpdateStrategy(userConfig, status.GetOsVersion()); err != nil {
- return fmt.Errorf("Error resolving update strategy (%s). Instance config will not be applied.", err)
+ return fmt.Errorf("error resolving update strategy (%s). Instance config will not be applied.", err)
}
if isUpdateDisabled(userConfig) {
glog.Info("Detected updates disabled: stopping update engine...")
if err := stopUpdateEngine(false); err != nil {
- glog.Errorf("Failed to stop update engine: %v", err)
+ glog.Errorf("failed to stop update engine: %v", err)
}
}
if err = switchChannelIfNeeded(ueClient, status, userConfig); err != nil {
- return fmt.Errorf("Error switching channel (%s). Instance config will not be applied.", err)
+ return fmt.Errorf("error switching channel (%s). Instance config will not be applied.", err)
}
return manager.SetInstanceConfig(userConfig)
@@ -313,11 +313,11 @@
channelConfig := fmt.Sprintf("%s-channel", targetChannel)
if err := ueClient.SetChannel(channelConfig); err != nil {
- glog.Errorf("Channel switch from %s to %s failed: %s.",
+ glog.Errorf("channel switch from %s to %s failed: %s.",
currentChannel, targetChannel, err)
return err
} else {
- glog.Infof("Channel switch from %s to %s succeeded.",
+ glog.Infof("channel switch from %s to %s succeeded.",
currentChannel, targetChannel)
}
}
@@ -332,7 +332,7 @@
match := imageNamePattern.FindStringSubmatch(strategy)
if match == nil {
- return nil, fmt.Errorf("Error parsing update_strategy: %s", strategy)
+ return nil, fmt.Errorf("error parsing update_strategy: %s", strategy)
}
channelValue := policymanagerproto.ReleaseChannel_value[strings.ToUpper(match[2])]
@@ -355,7 +355,7 @@
if ueStatus.UpdateStatus == "UPDATE_STATUS_UPDATED_NEED_REBOOT" {
config, err := manager.GetInstanceConfig()
if err != nil {
- return fmt.Errorf("Failed to get Instance config: %v, "+
+ return fmt.Errorf("failed to get Instance config: %v, "+
"ignore UPDATED_NEED_REBOOT", err)
}
@@ -368,11 +368,11 @@
glog.Infof("New COS version %v has been installed. Rebooting...", ueStatus.NewVersion)
_, _, err := apiHandler.RunCommand("/sbin/shutdown", "-r", "now")
if err != nil {
- return fmt.Errorf("Reboot attempt failed: %v", err)
+ return fmt.Errorf("reboot attempt failed: %v", err)
}
} else {
return fmt.Errorf(
- "Installed new version %v doesn't match "+
+ "installed new version %v doesn't match "+
"instance config target_version_prefix %v, "+
"ignore UPDATED_NEED_REBOOT",
ueStatus.NewVersion, config.GetTargetVersionPrefix())
@@ -395,7 +395,7 @@
glog.Info("Detected updates disabled: stopping update engine...")
// Use --no-block because update-engine depends on init-device-policy
if err := stopUpdateEngine(true); err != nil {
- glog.Errorf("Failed to stop update-engine: %v", err)
+ glog.Errorf("failed to stop update-engine: %v", err)
}
}
@@ -606,7 +606,7 @@
case showConfigCommand:
handleShowConfigCmd(args)
default:
- glog.Errorf("Unrecognized command: %s\n", command)
+ glog.Errorf("unrecognized command: %s\n", command)
flag.Usage()
}
diff --git a/src/policy_manager/policymanagerproto/instance_status.proto b/src/policy_manager/policymanagerproto/instance_status.proto
index ac0f511..1c30681 100644
--- a/src/policy_manager/policymanagerproto/instance_status.proto
+++ b/src/policy_manager/policymanagerproto/instance_status.proto
@@ -15,6 +15,8 @@
// Protobuf definitions for reporting instance status.
syntax = "proto2";
+import "policy_manager/policymanagerproto/update_status.proto";
+
package policymanagerproto;
// ReleaseChannel specifies the Chrome OS release channel that the image is on.
@@ -124,7 +126,7 @@
}
// The current update status as reported by the update_engine.
- optional UpdateStatus update_status = 3 [default = UPDATE_STATUS_UNKNOWN];
+ optional Operation update_status = 3 [default = ERROR];
// New version delivered by the update.
// This fields is only useful if the update engine status is one of the
diff --git a/src/policy_manager/policymanagerproto/update_status.proto b/src/policy_manager/policymanagerproto/update_status.proto
new file mode 100644
index 0000000..3e06a8f
--- /dev/null
+++ b/src/policy_manager/policymanagerproto/update_status.proto
@@ -0,0 +1,98 @@
+// Copyright 2022 Google LLC
+//
+// Licensed 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.
+
+// Protobuf definitions for Update Status of COS instance. Keep this file
+// synced with
+// chromiumos/src/platform2/system_api/dbus/update_engine/update_engine.proto.
+syntax = "proto2";
+
+package policymanagerproto;
+
+// Keep in sync with:
+// update_engine/client_library/include/update_engine/update_status.h
+enum Operation {
+ IDLE = 0;
+ CHECKING_FOR_UPDATE = 1;
+ UPDATE_AVAILABLE = 2;
+ DOWNLOADING = 3;
+ VERIFYING = 4;
+ FINALIZING = 5;
+ UPDATED_NEED_REBOOT = 6;
+ REPORTING_ERROR_EVENT = 7;
+ ATTEMPTING_ROLLBACK = 8;
+ DISABLED = 9;
+ NEED_PERMISSION_TO_UPDATE = 10;
+
+ // ERROR is only used by Chrome and update_engine doesn't really use/set this
+ // value as it is not the proper way to propagate errors. DO NOT use this
+ // anywhere other than Chrome.
+ // TODO(crbug.com/977320): Remove this from Chrome.
+ ERROR = -1;
+}
+
+// Keep in sync with:
+// update_engine/client_library/include/update_engine/update_status.h
+enum UpdateUrgency {
+ REGULAR = 0;
+ CRITICAL = 1;
+}
+
+// This is the message transferred between update_engine and other processes
+// about the current status of the update_engine. It is used either in
+// |GetUpdateStatusAdvanced| DBus method or |StatusUpdateAdvanced| signal.
+//
+// NOTE: Keep this updated with:
+// update_engine/client_library/include/update_engine/update_status.h
+message StatusResult {
+ // When the update_engine last checked for updates (time_t: seconds from Unix
+ // epoch).
+ optional int64 last_checked_time = 1;
+
+ // The current progress (0.0f-1.0f).
+ optional double progress = 2;
+
+ // The current status/operation of the update_engine.
+ optional Operation update_status = 3;
+
+ // The new product version.
+ optional string new_version = 4;
+
+ // The size of the update payload (bytes).
+ optional int64 new_size = 5;
+
+ // Whether the update is actually an enterprise rollback. The value is valid
+ // only if the current_operation is passed |CHECKING_FOR_UPDATE|.
+ optional bool is_enterprise_rollback = 6;
+
+ // Indication of install for DLC(s).
+ optional bool is_install = 7;
+
+ // The end-of-life date of the device in the number of days since Unix Epoch.
+ optional int64 eol_date = 8;
+
+ // If true, the system will powerwash once the update is applied and the
+ // system is rebooted. This value is reliable on |UPDATE_NEED_REBOOT|.
+ optional bool will_powerwash_after_reboot = 9;
+
+ // The last update/install attempt error.
+ // Reference update_engine/dbus-constants.h for the errors that update_engine
+ // will propagate into this field.
+ optional int32 last_attempt_error = 10;
+
+ // If true, the update is critical. DEPRECATED.
+ // bool critical_update = 11;
+
+ // Indicates how important an update is.
+ optional UpdateUrgency update_urgency = 12;
+}
diff --git a/src/policy_manager/policymanagerutil/config_fetcher.go b/src/policy_manager/policymanagerutil/config_fetcher.go
index 2c4d03d..33422a7 100644
--- a/src/policy_manager/policymanagerutil/config_fetcher.go
+++ b/src/policy_manager/policymanagerutil/config_fetcher.go
@@ -114,16 +114,16 @@
var err error
if rawMetadata, etag, err = retriever.FetchMetadata(lastEtag); err != nil {
- glog.Errorf("Failed to fetch metadata from metadata server: %s", err)
+ glog.Errorf("failed to fetch metadata from metadata server: %s", err)
return nil, "", err
}
if metadata, err = parseRawMetadata(rawMetadata); err != nil {
- glog.Errorf("Failed to parse raw metadata: %s", err)
+ glog.Errorf("failed to parse raw metadata: %s", err)
return nil, "", err
}
- glog.V(4).Infof("Current value of instance custom metadata directory: %v", metadata)
+ glog.V(4).Infof("current value of instance custom metadata directory: %v", metadata)
userConfig := getUserConfigFromMetadata(metadata)
return userConfig, etag, nil
}
@@ -165,7 +165,7 @@
if configStr, ok = metadata[gciLegacyConfigKey]; ok {
if configStr != "" {
if err = jsonpb.UnmarshalString(configStr, userConfig); err != nil {
- glog.Errorf("Failed to unmarshal InstanceConfig %s: %s", configStr, err)
+ glog.Errorf("failed to unmarshal InstanceConfig %s: %s", configStr, err)
}
}
return userConfig
