Skip to content

Commit dfc4c52

Browse files
pothospothos
committed
Only store Omaha XML response for update check
The full Omaha XML response is stored to a file for the postinst action, but we only want the response for the initial update check passed this way and not overwrite the file with responses for state reports. Only store the Omaha XML response for the update check and not all of the interaction with the Omaha server.
1 parent a9b9bee commit dfc4c52

File tree

6 files changed

+35
-14
lines changed

6 files changed

+35
-14
lines changed

‎flatcar-postinst

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ if [ "${OEMID}" != "" ] && { [ -e "${INSTALL_MNT}/share/flatcar/oems/${OEMID}" ]
135135
# which only works with a new update-engine client that creates "full-response",
136136
# and we also have to check that this file was created fresh for this update operation
137137
# (relies on the reset of /var/lib/update_engine/prefs/previous-version that old clients also do)
138-
if [ -e /var/lib/update_engine/prefs/full-response ] && [ $(stat -L --printf='%Y' /var/lib/update_engine/prefs/full-response) -gt $(stat -L --printf='%Y' /var/lib/update_engine/prefs/previous-version) ]; then
138+
if [ -e /var/lib/update_engine/prefs/full-response ] && [ $(stat -L --printf='%Y' /var/lib/update_engine/prefs/full-response) -ge $(stat -L --printf='%Y' /var/lib/update_engine/prefs/previous-version) ]; then
139139
rm -f "/var/lib/update_engine/oem-${OEMID}.raw"
140140
sysext_download "oem-${OEMID}.gz" "/var/lib/update_engine/oem-${OEMID}.raw" /var/lib/update_engine/prefs/full-response
141141
fi
@@ -148,6 +148,7 @@ if [ "${OEMID}" != "" ] && { [ -e "${INSTALL_MNT}/share/flatcar/oems/${OEMID}" ]
148148
PAYLOADSERVER=bincache-server
149149
PAYLOADNAME="flatcar_test_update-oem-${OEMID}.gz"
150150
fi
151+
echo "Falling back to ${PAYLOADSERVER} for extension 'oem-${OEMID}'" >&2
151152
sysext_download "${PAYLOADNAME}" "/var/lib/update_engine/oem-${OEMID}.raw" "${PAYLOADSERVER}"
152153
fi
153154
if [ "${SUCCESS}" = false ]; then
@@ -194,7 +195,7 @@ for NAME in $(grep -h -o '^[^#]*' /etc/flatcar/enabled-sysext.conf /usr/share/fl
194195
# which only works with a new update-engine client that creates "full-response",
195196
# and we also have to check that this file was created fresh for this update operation
196197
# (relies on the reset of /var/lib/update_engine/prefs/previous-version that old clients also do)
197-
if [ -e /var/lib/update_engine/prefs/full-response ] && [ $(stat -L --printf='%Y' /var/lib/update_engine/prefs/full-response) -gt $(stat -L --printf='%Y' /var/lib/update_engine/prefs/previous-version) ]; then
198+
if [ -e /var/lib/update_engine/prefs/full-response ] && [ $(stat -L --printf='%Y' /var/lib/update_engine/prefs/full-response) -ge $(stat -L --printf='%Y' /var/lib/update_engine/prefs/previous-version) ]; then
198199
rm -f "/var/lib/update_engine/flatcar-${NAME}.raw"
199200
sysext_download "flatcar-${NAME}.gz" "/var/lib/update_engine/flatcar-${NAME}.raw" /var/lib/update_engine/prefs/full-response
200201
fi
@@ -207,6 +208,7 @@ for NAME in $(grep -h -o '^[^#]*' /etc/flatcar/enabled-sysext.conf /usr/share/fl
207208
PAYLOADSERVER=bincache-server
208209
PAYLOADNAME="flatcar_test_update-flatcar-${NAME}.gz"
209210
fi
211+
echo "Falling back to ${PAYLOADSERVER} for extension 'flatcar-${NAME}'" >&2
210212
sysext_download "${PAYLOADNAME}" "/var/lib/update_engine/flatcar-${NAME}.raw" "${PAYLOADSERVER}"
211213
fi
212214
if [ "${SUCCESS}" = false ]; then

‎src/update_engine/omaha_request_action.cc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,13 +243,15 @@ string XmlEncode(const string& input) {
243243
OmahaRequestAction::OmahaRequestAction(SystemState* system_state,
244244
OmahaEvent* event,
245245
HttpFetcher* http_fetcher,
246-
bool ping_only)
246+
bool ping_only,
247+
bool store_response)
247248
: system_state_(system_state),
248249
event_(event),
249250
http_fetcher_(http_fetcher),
250251
ping_only_(ping_only),
251252
ping_active_days_(0),
252-
ping_roll_call_days_(0) {
253+
ping_roll_call_days_(0),
254+
store_response_(store_response) {
253255
params_ = system_state->request_params();
254256
}
255257

@@ -614,8 +616,10 @@ void OmahaRequestAction::TransferComplete(HttpFetcher *fetcher,
614616
string current_response(response_buffer_.begin(), response_buffer_.end());
615617
LOG(INFO) << "Omaha request response: " << current_response;
616618

617-
LOG_IF(WARNING, !system_state_->prefs()->SetString(kPrefsFullResponse, current_response))
618-
<< "Unable to write full response.";
619+
if (store_response_) {
620+
LOG_IF(WARNING, !system_state_->prefs()->SetString(kPrefsFullResponse, current_response))
621+
<< "Unable to write full response.";
622+
}
619623

620624
// Events are best effort transactions -- assume they always succeed.
621625
if (IsEvent()) {

‎src/update_engine/omaha_request_action.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ class OmahaRequestAction : public Action<OmahaRequestAction>,
115115
OmahaRequestAction(SystemState* system_state,
116116
OmahaEvent* event,
117117
HttpFetcher* http_fetcher,
118-
bool ping_only);
118+
bool ping_only,
119+
bool store_response);
119120
virtual ~OmahaRequestAction();
120121
typedef ActionTraits<OmahaRequestAction>::InputObjectType InputObjectType;
121122
typedef ActionTraits<OmahaRequestAction>::OutputObjectType OutputObjectType;
@@ -204,6 +205,9 @@ class OmahaRequestAction : public Action<OmahaRequestAction>,
204205
int ping_active_days_;
205206
int ping_roll_call_days_;
206207

208+
// If true, store full response to XML dump file for the postinst action.
209+
bool store_response_;
210+
207211
DISALLOW_COPY_AND_ASSIGN(OmahaRequestAction);
208212
};
209213

‎src/update_engine/omaha_request_action_unittest.cc

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,8 @@ bool TestUpdateCheck(PrefsInterface* prefs,
137137
OmahaRequestAction action(&mock_system_state,
138138
NULL,
139139
fetcher,
140-
ping_only);
140+
ping_only,
141+
false);
141142
processor.EnqueueAction(&action);
142143

143144
ObjectCollectorAction<OmahaResponse> collector_action;
@@ -169,7 +170,7 @@ void TestEvent(OmahaRequestParams params,
169170
ActionProcessor processor;
170171
ActionTestDelegate<OmahaRequestAction> delegate;
171172

172-
OmahaRequestAction action(&mock_system_state, event, fetcher, false);
173+
OmahaRequestAction action(&mock_system_state, event, fetcher, false, false);
173174
processor.EnqueueAction(&action);
174175

175176
delegate.RunProcessorInMainLoop(&processor);
@@ -230,6 +231,7 @@ TEST(OmahaRequestActionTest, NoOutputPipeTest) {
230231
OmahaRequestAction action(&mock_system_state, NULL,
231232
new MockHttpFetcher(http_response.data(),
232233
http_response.size()),
234+
false,
233235
false);
234236
processor.EnqueueAction(&action);
235237

@@ -412,6 +414,7 @@ TEST(OmahaRequestActionTest, TerminateTransferTest) {
412414
OmahaRequestAction action(&mock_system_state, NULL,
413415
new MockHttpFetcher(http_response.data(),
414416
http_response.size()),
417+
false,
415418
false);
416419
TerminateEarlyTestProcessorDelegate delegate;
417420
delegate.loop_ = loop;
@@ -517,7 +520,7 @@ TEST(OmahaRequestActionTest, FormatUpdateCheckOutputTest) {
517520
NiceMock<PrefsMock> prefs;
518521
EXPECT_CALL(prefs, GetString(kPrefsPreviousVersion, _))
519522
.WillOnce(DoAll(SetArgumentPointee<1>(string("")), Return(true)));
520-
EXPECT_CALL(prefs, SetString(kPrefsFullResponse, _)).Times(1);
523+
EXPECT_CALL(prefs, SetString(kPrefsFullResponse, _)).Times(0);
521524
EXPECT_CALL(prefs, SetString(kPrefsPreviousVersion, _)).Times(1);
522525
ASSERT_FALSE(TestUpdateCheck(&prefs,
523526
GetDefaultTestParams(),
@@ -586,6 +589,7 @@ TEST(OmahaRequestActionTest, IsEventTest) {
586589
NULL,
587590
new MockHttpFetcher(http_response.data(),
588591
http_response.size()),
592+
false,
589593
false);
590594
EXPECT_FALSE(update_check_action.IsEvent());
591595

@@ -596,6 +600,7 @@ TEST(OmahaRequestActionTest, IsEventTest) {
596600
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
597601
new MockHttpFetcher(http_response.data(),
598602
http_response.size()),
603+
false,
599604
false);
600605
EXPECT_TRUE(event_action.IsEvent());
601606
}

‎src/update_engine/update_attempter.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ void UpdateAttempter::BuildUpdateActions(bool interactive) {
188188
new OmahaRequestAction(system_state_,
189189
NULL,
190190
update_check_fetcher, // passes ownership
191-
false));
191+
false,
192+
true)); // Store full response for the update check
192193
shared_ptr<OmahaResponseHandlerAction> response_handler_action(
193194
new OmahaResponseHandlerAction(system_state_));
194195
shared_ptr<FilesystemCopierAction> filesystem_copier_action(
@@ -199,6 +200,7 @@ void UpdateAttempter::BuildUpdateActions(bool interactive) {
199200
new OmahaEvent(
200201
OmahaEvent::kTypeUpdateDownloadStarted),
201202
new LibcurlHttpFetcher(),
203+
false,
202204
false));
203205
LibcurlHttpFetcher* download_fetcher = new LibcurlHttpFetcher();
204206
download_fetcher->set_check_certificate(CertificateChecker::kDownload);
@@ -214,6 +216,7 @@ void UpdateAttempter::BuildUpdateActions(bool interactive) {
214216
new OmahaEvent(
215217
OmahaEvent::kTypeUpdateDownloadFinished),
216218
new LibcurlHttpFetcher(),
219+
false,
217220
false));
218221
shared_ptr<FilesystemCopierAction> filesystem_verifier_action(
219222
new FilesystemCopierAction(true));
@@ -227,6 +230,7 @@ void UpdateAttempter::BuildUpdateActions(bool interactive) {
227230
new OmahaRequestAction(system_state_,
228231
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
229232
new LibcurlHttpFetcher(),
233+
false,
230234
false));
231235

232236
download_action->set_delegate(this);
@@ -620,6 +624,7 @@ bool UpdateAttempter::ScheduleErrorEventAction() {
620624
new OmahaRequestAction(system_state_,
621625
error_event_.release(), // Pass ownership.
622626
new LibcurlHttpFetcher(),
627+
false,
623628
false));
624629
actions_.push_back(shared_ptr<AbstractAction>(error_event_action));
625630
processor_->EnqueueAction(error_event_action.get());
@@ -690,7 +695,8 @@ void UpdateAttempter::PingOmaha() {
690695
new OmahaRequestAction(system_state_,
691696
NULL,
692697
new LibcurlHttpFetcher(),
693-
true));
698+
true,
699+
false));
694700
actions_.push_back(shared_ptr<OmahaRequestAction>(ping_action));
695701
processor_->set_delegate(NULL);
696702
processor_->EnqueueAction(ping_action.get());

‎src/update_engine/update_attempter_unittest.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ TEST_F(UpdateAttempterTest, ActionCompletedOmahaRequestTest) {
122122
std::unique_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0));
123123
fetcher->FailTransfer(500); // Sets the HTTP response code.
124124
OmahaRequestAction action(&mock_system_state_, NULL,
125-
fetcher.release(), false);
125+
fetcher.release(), false, false);
126126
ObjectCollectorAction<OmahaResponse> collector_action;
127127
BondActions(&action, &collector_action);
128128
OmahaResponse response;
@@ -168,7 +168,7 @@ TEST_F(UpdateAttempterTest, GetErrorCodeForActionTest) {
168168

169169
MockSystemState mock_system_state;
170170
OmahaRequestAction omaha_request_action(&mock_system_state, NULL,
171-
NULL, false);
171+
NULL, false, false);
172172
EXPECT_EQ(kActionCodeOmahaRequestError,
173173
GetErrorCodeForAction(&omaha_request_action, kActionCodeError));
174174
OmahaResponseHandlerAction omaha_response_handler_action(&mock_system_state_);

0 commit comments

Comments
 (0)