Re: [CCAMP] Roman Danyliw's Discuss on draft-ietf-ccamp-alarm-module-08: (with DISCUSS and COMMENT)

[Martin Bjorklund <[e-mail]>]

Hi,

Thank you for this review.  See comments inline.


Roman Danyliw via Datatracker <[e-mail]> wrote:
> Roman Danyliw has entered the following ballot position for
> draft-ietf-ccamp-alarm-module-08: Discuss
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-ccamp-alarm-module/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> (1) Section 3.5, Alarm Life-Cycle.  The text states that “A server
> SHOULD describe how long it retains cleared/closed alarms: until
> manually purged or if it has an automatic removal policy.” How is
> this retention policy described?  Is that in scope for this
> document?

You are right, this is not in scope.  We suggest we add a sentence:

  How this is done is outside the scope of this document.


> (2) Section 4.2, Alarm Inventory.  The text states that “A server
> MUST implement the alarm inventory in order to enable controlled
> alarm procedures in the client.” What is the expected server
> behavior if a client sends an alarm type not in the inventory (and
> it isn’t part of the dynamic addition process)?

We assume you mean what does a management application do if it
receives an alarm from a device that is not in the inventory?  This is
the reason for the MUST in the text; a device MUST list all alarm
types in the inventory so that a management application knows about
it.

> (2) Section 10, Security Considerations.  It seems like
> “/alarms/alarm-list/alarm/set-operator-state” should be listed as an operation
> in the YANG model that presents a security issues (just like “purge-alarms”). 
> Consider if one altered the operator alert state causing the alarm management
> procedures to miss an alert (e.g., setting an alert to “closed” before any
> action is taken).


You are right. We suggest:

   /alarms/alarm-list/alarm/set-operator-state:  This action can be used
      by the operator to indicate the level of human intervention on an
      alarm.  Unauthorized use of this action could result in alarms
      being ignored by operators.


> (3) Section 10, Security Considerations.  I don’t know must about the
> implementations, but wouldn’t compressing alerts (per compress-alarms and
> compress-shelved-alarms operations) remove them from consideration by alarm
> management procedures?  If so, these would be a sensitive operation that would
> need to be listed as the concern equivalent to the current text for
> purge-alarms.

Compressing only affects the history (old states) of the alarm.  The
alarm itself is not affected, so we don't think this needs additional
considerations.

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> (1) Section 1.1, Terminology, “Fault”.  Consider expanding the acronym “MOS”
> (Mean Option Score?)

Done - (Mean Opinion Score).

> (2) Section 2, Objectives, Consider s/X.733/[X.733]/

Done.

> (3) Section 3.2, Alarm Type, Consider s/identity based/identity-based/

Done.

> (4) Section 3.2, Alarm Type, Typo, s/standard organization/standards
> organization/

Done.

> (5) Section 3.4, Identifying Alarm Instances, Consider s/were not really
> clear/were not clear/

Done.

> (6) Section 3.5.2, Operator Alarm Life-cycle, Consider s/can also act upon/act
> upon/

We suggest "Operators can act upon..."  (removed "also").

> (7) Section 3.5.2, Operator Alarm Life-cycle, Consider s/A closed alarm is an
> alarm/For example, a closed alarm is an alarm/

Done.

> (8) Section 3.6, Root Cause, Impacted Resources and Related Alarms, Consider
> s/Different systems have various various/Different systems have varying/

Done.

> (9) Section 3.6, Root Cause, Impacted Resources and Related Alarms, Consider
> s/In some occasions/On some occasions/

Done.

> (10) Section 3.6, Root Cause, Impacted Resources and Related Alarms, Consider
> s/needs to represent an alarm that indicates a situation that needs acting
> upon/raises an alarm to indicate a situation requiring attention/

Done.

> (11) Section 4.1.1, Alarm Shelving, The text states “The instrumentation MUST
> move shelved alarms from the alarm list (/alarms/alarm-list) to the shelved
> alarm list (/alarms/shelved-alarms/).”  It wasn’t clear when these shelved
> alarms must be moved given the text.

You are right.  Actually, the word "move" is a bit misleading.  We suggest:

OLD:

   Shelved alarms are shown in a dedicated shelved alarm list.  The
   instrumentation MUST move shelved alarms from the alarm list
   (/alarms/alarm-list) to the shelved alarm list (/alarms/shelved-
   alarms/).  Shelved alarms do not generate any notifications.  When
   the shelving criteria is removed or changed the alarm list MUST be
   updated to the correct actual state of the alarms.

NEW:

   Shelved alarms are shown in a dedicated shelved alarm list.  Matching
   alarms MUST appear in the /alarms/shelved-alarms/shelved-alarm list,
   and non-matching /alarms MUST appear in the /alarms/alarm-list/alarm
   list.  The server does not send any notifications for shelved alarms.

(and the same change in the YANG module)


> (12) Section 4.4, The Alarm List, The sentence, “The alarm list
> (/alarms/alarm-list) is a function from (resource, alarm type, alarm type
> qualifier) to the current composite alarm state” is missing a word/phrase. 
> Removing the parenthetical remarks it reads a “The alarm list is a function
> from to the current composite alarm state” is does not parse.

Ok, we suggest:

     The alarm list (/alarms/alarm-list) is a function from the tuple
     (resource, alarm type, alarm type qualifier) to the current
     composite alarm state.


> (13) Consider s/Life-cycle/Lifecycle/g

Done.

Thanks again for this review!


/martin & stefan