Merge lp://staging/~sophia-wu/opencompute/add-ocp-system-event-log-job into lp://staging/opencompute/checkbox

Proposed by Sophia Wu
Status: Merged
Approved by: Jeff Lane 
Approved revision: 2174
Merged at revision: 2173
Proposed branch: lp://staging/~sophia-wu/opencompute/add-ocp-system-event-log-job
Merge into: lp://staging/opencompute/checkbox
Diff against target: 459 lines (+230/-79)
8 files modified
data/whitelists/opencompute-certify-remoteME.whitelist (+46/-0)
debian/changelog (+0/-79)
debian/checkbox.install (+1/-0)
debian/checkbox.postinst (+1/-0)
examples/me.cfg (+15/-0)
jobs/TC-002-0011-System_Log.txt.in (+11/-0)
jobs/local.txt.in (+9/-0)
scripts/ipmi_sel_entries (+147/-0)
To merge this branch: bzr merge lp://staging/~sophia-wu/opencompute/add-ocp-system-event-log-job
Reviewer Review Type Date Requested Status
Jeff Lane  Approve
Review via email: mp+213419@code.staging.launchpad.net

This proposal supersedes a proposal from 2014-02-14.

Description of the change

Verify the system event log must be capable of saving at least 256 entries

To post a comment you must log in.
Revision history for this message
Jeff Lane  (bladernr) wrote : Posted in a previous version of this proposal

Hi Sophia.... thanks for submitting this. In general it looks good but there are a couple things:

1: Why is there a separate whitelist for this? Does this need it's own whitelist, or would this be in a general certification remote whitelist? If that's the case, the whitelist should simply be called opencompute-certify-remote.whitelist.

2: if you want the config file me.cfg to be installed, you need to add it to the approrpiate files in the debian directory. here are teh places where the examples directory is referenced:

debian/checkbox-hw-collection.install:usr/share/checkbox/examples/checkbox-hw-collection.ini
debian/checkbox.install:usr/share/checkbox/examples/checkbox.ini
debian/checkbox.install:usr/share/checkbox/examples/network.cfg
debian/checkbox.install:usr/share/checkbox/examples/virtualization.cfg
debian/checkbox.install:usr/share/checkbox/examples/org.freedesktop.policykit.checkbox.policy usr/share/polkit-1/actions/
debian/checkbox-ocp-cli.install:usr/share/checkbox/examples/checkbox-ocp-cli.ini
debian/checkbox-ocp-gtk.install:usr/share/checkbox/examples/checkbox-ocp-gtk.ini
debian/checkbox-ocp-qt.install:usr/share/checkbox/examples/checkbox-ocp-qt.ini
debian/checkbox-ocp-urwid.install:usr/share/checkbox/examples/checkbox-ocp-urwid.ini
debian/checkbox.postinst:cp /usr/share/checkbox/examples/network.cfg /etc/checkbox.d/
debian/checkbox.postinst:cp /usr/share/checkbox/examples/virtualization.cfg /etc/checkbox.d/

You're probably interested in checkbox.install and checkbox.postinst

Without adding it to those files, the config file will not be installed with the package.

3: in the ipmi_sel_entries script, you reference ../examples/me.cfg twice. The proper installed location is /etc/checkbox.d/ (See 2 above).

4: Finally, you also need to update debian/changelog and add an entry under your name that briefly describes the changes you've made. See other entries in the changelog for examples.

Fix those few items (and just let me know more about item 1) and I'll push this through.

Thanks!

review: Needs Fixing
Revision history for this message
Sophia Wu (sophia-wu) wrote : Posted in a previous version of this proposal

Hi Jeff,

Thanks for your feedback. It's very helpful for me.

And about item 1,
Due to some other test cases that we haven't commit now, they will run ME stress test.
I think it's better to separate the whitelists, one is for retrieving ME data and another is for stressing ME.

Thanks,
Sophia

Revision history for this message
Jeff Lane  (bladernr) wrote : Posted in a previous version of this proposal

Hi Sophia...

I'm ready to pass this through and merge it, but I need one additional thing from you.

Whenver you do a merge request, please also update debian/changelog

See the current changelog for examples... So I need you to do the following:

1: update your branch to the latest trunk code using bzr pull:

cd your/local/branch/dir
bzr pull #should update your branch with the latest trunk

edit /your/local/branch/dir/debian/changelog

Look at Nelson's entry and add a similar one to that below his... so ti should look something like this:

[Nelson Chu]
* Item 1
* Item 2
* Itme 3

[Sophia Wu]
* Item 1
* Item 2

If you can do that and resubmit, I'l approve and merge your branch as soon as I see the update.

Thanks!

Jeff

review: Needs Fixing
Revision history for this message
Jeff Lane  (bladernr) wrote : Posted in a previous version of this proposal

Thanks... and in the future, if I haven't gotten to something like this within a couple days, please ping me directly... sometimes these notices get lost in the noise (I get better than 1000 emails a day, most of whcih are automated from Launchpad).

review: Approve
Revision history for this message
Jeff Lane  (bladernr) wrote : Posted in a previous version of this proposal

Attempt to merge into lp:opencompute/checkbox failed due to conflicts:

duplicate in debian/changelog.moved
text conflict in jobs/local.txt.in

Revision history for this message
Jeff Lane  (bladernr) wrote : Posted in a previous version of this proposal

OK, so tarmac tried to merge the code and ran into some conflicts...

First, why did you add a brand new copy of the changelog, or more importantly, how did you manage to do that? You don't need to create a new changelog, simple edit the existing one...

So lets focus on getting this cleared up... do the following:

first, pull a fresh copy of the checkbox code:

bzr branch lp:opencompute/checkbox ocp-checkbox

Next, merge YOUR branch into that:

cd ocp-checkbox
bzr merge lp:~sophia-wu/opencompute/add-ocp-system-event-log-job

And you should see the following:
+N data/whitelists/opencompute-certify-remoteME.whitelist
+N debian/changelog
+N examples/me.cfg
+N jobs/TC-002-0011-System_Log.txt.in
+N scripts/ipmi_sel_entries
R debian/changelog => debian/changelog.moved
 M debian/checkbox.install
 M debian/checkbox.postinst
 M jobs/local.txt.in
Conflict adding file debian/changelog. Moved existing file to debian/changelog.moved.
Text conflict in jobs/local.txt.in
2 conflicts encountered.

The conflicts are what held this up. So first, fix the changelog. As I mentioned above, for some reason, it thinks you've added a brand new copy of the changelog. So we need to do the following:

cd debian
mv changelog changelog.sophia
mv changelog.moved changelog

then take the stuff you added (the Sophia section of changelog.sophia) and add that to changelog.

then delete changelog.sophia

rm changelog.sophia

Now you should be able to resolve the first conflict:

bzr resolve debian/changelog

which should give you this:
1 conflict resolved, 1 remaining

Now run bzr conflicts to see what's left:
Text conflict in jobs/local.txt.in

So lets now look at jobs/local.txt.in. The conflict is at the end:

<<<<<<< TREE
name: __TC-003-0001-Hardware_Information__
=======
name: __TC-002-0011-System_Log__
>>>>>>> MERGE-SOURCE
plugin: local
<<<<<<< TREE
_description: Verify hardware information
=======
description: System Event Log
>>>>>>> MERGE-SOURCE
command:
<<<<<<< TREE
  shopt -s extglob
  cat $CHECKBOX_SHARE/jobs/TC-003-0001-Hardware_Information.txt?(.in)
=======
 shopt -s extglob
 cat $CHECKBOX_SHARE/jobs/TC-002-0011-System_Log.txt?(.in)

>>>>>>> MERGE-SOURCE

So clean that up to look correct:

name: __TC-001-0002-Platform_Controller_Hub__
plugin: local
_description: Verify platform controller hub functionality
command:
 shopt -s extglob
 cat $CHECKBOX_SHARE/jobs/TC-001-0002-Platform_Controller_Hub.txt?(.in)

name: __TC-002-0011-System_Log__
plugin: local
description: System Event Log
command:
 shopt -s extglob
 cat $CHECKBOX_SHARE/jobs/TC-002-0011-System_Log.txt?(.in)

name: __TC-003-0001-Hardware_Information__
plugin: local
_description: Verify hardware information
command:
 shopt -s extglob
 cat $CHECKBOX_SHARE/jobs/TC-003-0001-Hardware_Information.txt?(.in)

Save and exit that file and run bzr resolve and you should see this:
All conflicts resolved.

then resubmit and I'll approve and get this merged.

review: Needs Fixing
2173. By Sophia Wu

resolve conflicts

2174. By Sophia Wu

Add OCP system event log job

Revision history for this message
Jeff Lane  (bladernr) wrote :

Thanks, lets try this again and see what happens.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches