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

[Roman Danyliw <[e-mail]>]

> -----Original Message-----
> From: iesg [mailto:[e-mail]] On Behalf Of Martin Bjorklund
> Sent: Monday, April 08, 2019 7:48 AM
> To: Roman Danyliw <[e-mail]>; [e-mail]
> Cc: [e-mail]; [e-mail]; ccamp-
> [e-mail]; [e-mail]
> Subject: Re: [CCAMP] Roman Danyliw's Discuss on draft-ietf-ccamp-alarm-
> module-08: (with DISCUSS and COMMENT)
> 
> 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.

Works for me.

> > (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?  

Yes, exactly.

> 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.

What if the device is misconfigured/rogue and doesn't implement the MUST (i.e., doesn't put all of the alarm types in the inventory; returns a different alarm type than requested by the server)?  I was expecting text roughly on the order of "if the management application gets an alarm of an unknown type it MUST discard 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.

Works for me.  Your editorial call on how to best sequence the operators: "alarms/alarm-list/set-operator-state" vs. "{alarms, alarm-list, set-operator-state}", "alarms, alarm-list, set-operator-state".

> > (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.

I missed that detail.  Agreed that no change it required.

> > ----------------------------------------------------------------------
> > 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.

All of these work for me.

Thanks,
Roman

> Thanks again for this review!
> 
> /martin & stefan