[jira] [Resolved] (KNOX-2146) Docs: Knox JWT token signature verification using public key
[ https://issues.apache.org/jira/browse/KNOX-2146?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sandeep More resolved KNOX-2146. Resolution: Fixed > Docs: Knox JWT token signature verification using public key > > > Key: KNOX-2146 > URL: https://issues.apache.org/jira/browse/KNOX-2146 > Project: Apache Knox > Issue Type: Bug > Components: Site >Affects Versions: 1.0.0 > Environment: Ubuntu 18.04, HDP 3.1 >Reporter: Matei C. >Assignee: Sandeep More >Priority: Minor > Fix For: 1.4.0 > > Attachments: knox_jwt_topo_apache_jira.txt, > knox_jwt_topo_apache_jira.txt, knox_jwt_topo_apache_jira.txt > > > Hello, > I have configured an Apache Knox (1.0.0) topology to accept 3rd party JWTs > by following this [Cloudera > guide|[https://community.cloudera.com/t5/Community-Articles/Knox-Accept-third-party-JWT/ta-p/248488]]. > > I would also like to verify the 3rd party JWts based on their signature by > adding my IdP's public key in PEM format for the JWT provider, but in the > guide it is specified that only PEM certificates are accepted (' [...] *In > current Knox version, public key is not supported, have to configure public > certificate [...]*') and I have not found any relevant documentation from > Knox on this subject. > > Can you please tell me if there is any solution to use public keys for JWT > verification in Knox 1.0.0 ? If not, are there any plans to support this in > future Knox releases ? > P.S.: > When adding the 'knox.token.verification.pem' parameter with the public key > in the JWT provider of my topology I noticed the below error in my > gateway.log, which does seem to confirm the public key limitation. > > {code:java} > javax.servlet.ServletException: javax.servlet.ServletException: > CertificateException - PEM may be corrupt > {code} > > Regards, > > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KNOX-2146) Docs: Knox JWT token signature verification using public key
[ https://issues.apache.org/jira/browse/KNOX-2146?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17078682#comment-17078682 ] Sandeep More commented on KNOX-2146: Added {{knox.token.verification.pem}} , {{sso.token.verification.pem}} and {{sso.expected.audiences}} to knox docs. https://knox.apache.org/books/knox-1-4-0/user-guide.html#JWT+Provider > Docs: Knox JWT token signature verification using public key > > > Key: KNOX-2146 > URL: https://issues.apache.org/jira/browse/KNOX-2146 > Project: Apache Knox > Issue Type: Bug > Components: Site >Affects Versions: 1.0.0 > Environment: Ubuntu 18.04, HDP 3.1 >Reporter: Matei C. >Assignee: Sandeep More >Priority: Minor > Fix For: 1.4.0 > > Attachments: knox_jwt_topo_apache_jira.txt, > knox_jwt_topo_apache_jira.txt, knox_jwt_topo_apache_jira.txt > > > Hello, > I have configured an Apache Knox (1.0.0) topology to accept 3rd party JWTs > by following this [Cloudera > guide|[https://community.cloudera.com/t5/Community-Articles/Knox-Accept-third-party-JWT/ta-p/248488]]. > > I would also like to verify the 3rd party JWts based on their signature by > adding my IdP's public key in PEM format for the JWT provider, but in the > guide it is specified that only PEM certificates are accepted (' [...] *In > current Knox version, public key is not supported, have to configure public > certificate [...]*') and I have not found any relevant documentation from > Knox on this subject. > > Can you please tell me if there is any solution to use public keys for JWT > verification in Knox 1.0.0 ? If not, are there any plans to support this in > future Knox releases ? > P.S.: > When adding the 'knox.token.verification.pem' parameter with the public key > in the JWT provider of my topology I noticed the below error in my > gateway.log, which does seem to confirm the public key limitation. > > {code:java} > javax.servlet.ServletException: javax.servlet.ServletException: > CertificateException - PEM may be corrupt > {code} > > Regards, > > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KNOX-2146) Docs: Knox JWT token signature verification using public key
[ https://issues.apache.org/jira/browse/KNOX-2146?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17078679#comment-17078679 ] ASF subversion and git services commented on KNOX-2146: --- Commit 1876305 from Sandeep More [ https://svn.apache.org/r1876305 ] KNOX-2146 - Add *.token.verification.pem property to documentation > Docs: Knox JWT token signature verification using public key > > > Key: KNOX-2146 > URL: https://issues.apache.org/jira/browse/KNOX-2146 > Project: Apache Knox > Issue Type: Bug > Components: Site >Affects Versions: 1.0.0 > Environment: Ubuntu 18.04, HDP 3.1 >Reporter: Matei C. >Assignee: Sandeep More >Priority: Minor > Fix For: 1.4.0 > > Attachments: knox_jwt_topo_apache_jira.txt, > knox_jwt_topo_apache_jira.txt, knox_jwt_topo_apache_jira.txt > > > Hello, > I have configured an Apache Knox (1.0.0) topology to accept 3rd party JWTs > by following this [Cloudera > guide|[https://community.cloudera.com/t5/Community-Articles/Knox-Accept-third-party-JWT/ta-p/248488]]. > > I would also like to verify the 3rd party JWts based on their signature by > adding my IdP's public key in PEM format for the JWT provider, but in the > guide it is specified that only PEM certificates are accepted (' [...] *In > current Knox version, public key is not supported, have to configure public > certificate [...]*') and I have not found any relevant documentation from > Knox on this subject. > > Can you please tell me if there is any solution to use public keys for JWT > verification in Knox 1.0.0 ? If not, are there any plans to support this in > future Knox releases ? > P.S.: > When adding the 'knox.token.verification.pem' parameter with the public key > in the JWT provider of my topology I noticed the below error in my > gateway.log, which does seem to confirm the public key limitation. > > {code:java} > javax.servlet.ServletException: javax.servlet.ServletException: > CertificateException - PEM may be corrupt > {code} > > Regards, > > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Resolved] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Larry McCay resolved KNOX-2304. --- Assignee: Larry McCay (was: Philip Zampino) Resolution: Fixed > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Larry McCay >Priority: Major > Fix For: 1.4.0 > > Time Spent: 3h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17078589#comment-17078589 ] ASF subversion and git services commented on KNOX-2304: --- Commit 89206511aad7cacb91c7fa032490816e7cdd18ba in knox's branch refs/heads/master from Larry McCay [ https://gitbox.apache.org/repos/asf?p=knox.git;h=8920651 ] KNOX-2304 - CM discovery cluster config monitor needs to be aware of … (#307) * KNOX-2304 - CM discovery cluster config monitor needs to be aware of all relevant CM event types Change-Id: Ic26ad36fcb110e01d30d636f14d5b383de01ff17 * KNOX-2304 - address review comments Change-Id: I5c2009f505b31c5c69ee7672ed37d21e1a7c48bb > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 3h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17078588#comment-17078588 ] ASF subversion and git services commented on KNOX-2304: --- Commit 89206511aad7cacb91c7fa032490816e7cdd18ba in knox's branch refs/heads/master from Larry McCay [ https://gitbox.apache.org/repos/asf?p=knox.git;h=8920651 ] KNOX-2304 - CM discovery cluster config monitor needs to be aware of … (#307) * KNOX-2304 - CM discovery cluster config monitor needs to be aware of all relevant CM event types Change-Id: Ic26ad36fcb110e01d30d636f14d5b383de01ff17 * KNOX-2304 - address review comments Change-Id: I5c2009f505b31c5c69ee7672ed37d21e1a7c48bb > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 3h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17078587#comment-17078587 ] ASF subversion and git services commented on KNOX-2304: --- Commit 89206511aad7cacb91c7fa032490816e7cdd18ba in knox's branch refs/heads/master from Larry McCay [ https://gitbox.apache.org/repos/asf?p=knox.git;h=8920651 ] KNOX-2304 - CM discovery cluster config monitor needs to be aware of … (#307) * KNOX-2304 - CM discovery cluster config monitor needs to be aware of all relevant CM event types Change-Id: Ic26ad36fcb110e01d30d636f14d5b383de01ff17 * KNOX-2304 - address review comments Change-Id: I5c2009f505b31c5c69ee7672ed37d21e1a7c48bb > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 3h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418788&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418788 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 18:38 Start Date: 08/Apr/20 18:38 Worklog Time Spent: 10m Work Description: lmccay commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418788) Time Spent: 3h (was: 2h 50m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 3h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] lmccay merged pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
lmccay merged pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418785&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418785 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 18:36 Start Date: 08/Apr/20 18:36 Worklog Time Spent: 10m Work Description: lmccay commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405732710 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: Not at this point. They won't be mixed up between the two and even if they were they both mean success. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418785) Time Spent: 2h 50m (was: 2h 40m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 2h 50m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405732710 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: Not at this point. They won't be mixed up between the two and even if they were they both mean success. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Updated] (KNOX-2320) Upgrade xmlsec to 2.1.5
[ https://issues.apache.org/jira/browse/KNOX-2320?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kevin Risden updated KNOX-2320: --- Resolution: Fixed Status: Resolved (was: Patch Available) > Upgrade xmlsec to 2.1.5 > --- > > Key: KNOX-2320 > URL: https://issues.apache.org/jira/browse/KNOX-2320 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Trivial > Fix For: 1.4.0 > > Attachments: KNOX-2320.patch > > > Upgrade xmlsec 2.1.4 to 2.1.5 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KNOX-2319) Upgrade commons-compress to 1.20
[ https://issues.apache.org/jira/browse/KNOX-2319?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17078579#comment-17078579 ] ASF subversion and git services commented on KNOX-2319: --- Commit 92b2ac4ea0fa8b3379136b5fb6e441a65547629a in knox's branch refs/heads/master from Kevin Risden [ https://gitbox.apache.org/repos/asf?p=knox.git;h=92b2ac4 ] KNOX-2319 - Upgrade commons-compress to 1.20 Signed-off-by: Kevin Risden > Upgrade commons-compress to 1.20 > > > Key: KNOX-2319 > URL: https://issues.apache.org/jira/browse/KNOX-2319 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Minor > Fix For: 1.4.0 > > Attachments: KNOX-2319.patch > > > Upgrade commons-compress 1.19 to 1.20 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KNOX-2319) Upgrade commons-compress to 1.20
[ https://issues.apache.org/jira/browse/KNOX-2319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kevin Risden updated KNOX-2319: --- Resolution: Fixed Status: Resolved (was: Patch Available) > Upgrade commons-compress to 1.20 > > > Key: KNOX-2319 > URL: https://issues.apache.org/jira/browse/KNOX-2319 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Minor > Fix For: 1.4.0 > > Attachments: KNOX-2319.patch > > > Upgrade commons-compress 1.19 to 1.20 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KNOX-2320) Upgrade xmlsec to 2.1.5
[ https://issues.apache.org/jira/browse/KNOX-2320?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17078580#comment-17078580 ] ASF subversion and git services commented on KNOX-2320: --- Commit d3b556b5c6e90f791151c593284853ccd787401f in knox's branch refs/heads/master from Kevin Risden [ https://gitbox.apache.org/repos/asf?p=knox.git;h=d3b556b ] KNOX-2320 - Upgrade xmlsec to 2.1.5 Signed-off-by: Kevin Risden > Upgrade xmlsec to 2.1.5 > --- > > Key: KNOX-2320 > URL: https://issues.apache.org/jira/browse/KNOX-2320 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Trivial > Fix For: 1.4.0 > > Attachments: KNOX-2320.patch > > > Upgrade xmlsec 2.1.4 to 2.1.5 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418777&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418777 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 18:22 Start Date: 08/Apr/20 18:22 Worklog Time Spent: 10m Work Description: pzampino commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405724123 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: It also may pick up _any_ event type that has a status value of `STARTED_STATUS` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418777) Time Spent: 2h 40m (was: 2.5h) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 2h 40m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405724123 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: It also may pick up _any_ event type that has a status value of `STARTED_STATUS` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418776&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418776 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 18:20 Start Date: 08/Apr/20 18:20 Worklog Time Spent: 10m Work Description: pzampino commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405723167 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: IIRC, && does have a higher precedence than ||, which could mean the default grouping is `START_COMMAND.equals(command) || (RESTART_COMMAND.equals(command) && SUCCEEDED_STATUS.equals(status)) || STARTED_STATUS.equals(status)`, which probably still works in this case This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418776) Time Spent: 2.5h (was: 2h 20m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 2.5h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405723167 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: IIRC, && does have a higher precedence than ||, which could mean the default grouping is `START_COMMAND.equals(command) || (RESTART_COMMAND.equals(command) && SUCCEEDED_STATUS.equals(status)) || STARTED_STATUS.equals(status)`, which probably still works in this case This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418774&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418774 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 18:16 Start Date: 08/Apr/20 18:16 Worklog Time Spent: 10m Work Description: pzampino commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405720842 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: I could see this being grouped as `(START_COMMAND.equals(command) && STARTED_STATUS.equals(status)) || (RESTART_COMMAND.equals(command) && SUCCEEDED_STATUS.equals(status))` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418774) Time Spent: 2h 20m (was: 2h 10m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 2h 20m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405720842 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: I could see this being grouped as `(START_COMMAND.equals(command) && STARTED_STATUS.equals(status)) || (RESTART_COMMAND.equals(command) && SUCCEEDED_STATUS.equals(status))` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Assigned] (KNOX-2146) Docs: Knox JWT token signature verification using public key
[ https://issues.apache.org/jira/browse/KNOX-2146?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sandeep More reassigned KNOX-2146: -- Assignee: Sandeep More (was: Larry McCay) > Docs: Knox JWT token signature verification using public key > > > Key: KNOX-2146 > URL: https://issues.apache.org/jira/browse/KNOX-2146 > Project: Apache Knox > Issue Type: Bug > Components: Site >Affects Versions: 1.0.0 > Environment: Ubuntu 18.04, HDP 3.1 >Reporter: Matei C. >Assignee: Sandeep More >Priority: Minor > Fix For: 1.4.0 > > Attachments: knox_jwt_topo_apache_jira.txt, > knox_jwt_topo_apache_jira.txt, knox_jwt_topo_apache_jira.txt > > > Hello, > I have configured an Apache Knox (1.0.0) topology to accept 3rd party JWTs > by following this [Cloudera > guide|[https://community.cloudera.com/t5/Community-Articles/Knox-Accept-third-party-JWT/ta-p/248488]]. > > I would also like to verify the 3rd party JWts based on their signature by > adding my IdP's public key in PEM format for the JWT provider, but in the > guide it is specified that only PEM certificates are accepted (' [...] *In > current Knox version, public key is not supported, have to configure public > certificate [...]*') and I have not found any relevant documentation from > Knox on this subject. > > Can you please tell me if there is any solution to use public keys for JWT > verification in Knox 1.0.0 ? If not, are there any plans to support this in > future Knox releases ? > P.S.: > When adding the 'knox.token.verification.pem' parameter with the public key > in the JWT provider of my topology I noticed the below error in my > gateway.log, which does seem to confirm the public key limitation. > > {code:java} > javax.servlet.ServletException: javax.servlet.ServletException: > CertificateException - PEM may be corrupt > {code} > > Regards, > > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KNOX-2319) Upgrade commons-compress to 1.20
[ https://issues.apache.org/jira/browse/KNOX-2319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kevin Risden updated KNOX-2319: --- Attachment: KNOX-2319.patch Status: Patch Available (was: In Progress) > Upgrade commons-compress to 1.20 > > > Key: KNOX-2319 > URL: https://issues.apache.org/jira/browse/KNOX-2319 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Minor > Fix For: 1.4.0 > > Attachments: KNOX-2319.patch > > > Upgrade commons-compress 1.19 to 1.20 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KNOX-2320) Upgrade xmlsec to 2.1.5
[ https://issues.apache.org/jira/browse/KNOX-2320?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kevin Risden updated KNOX-2320: --- Attachment: KNOX-2320.patch Status: Patch Available (was: In Progress) > Upgrade xmlsec to 2.1.5 > --- > > Key: KNOX-2320 > URL: https://issues.apache.org/jira/browse/KNOX-2320 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Trivial > Fix For: 1.4.0 > > Attachments: KNOX-2320.patch > > > Upgrade xmlsec 2.1.4 to 2.1.5 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work started] (KNOX-2320) Upgrade xmlsec to 2.1.5
[ https://issues.apache.org/jira/browse/KNOX-2320?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Work on KNOX-2320 started by Kevin Risden. -- > Upgrade xmlsec to 2.1.5 > --- > > Key: KNOX-2320 > URL: https://issues.apache.org/jira/browse/KNOX-2320 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Trivial > Fix For: 1.4.0 > > > Upgrade xmlsec 2.1.4 to 2.1.5 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KNOX-2319) Upgrade commons-compress to 1.20
[ https://issues.apache.org/jira/browse/KNOX-2319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kevin Risden updated KNOX-2319: --- Description: Upgrade commons-compress 1.19 to 1.20 (was: Upgrade commons-compress to 1.20) > Upgrade commons-compress to 1.20 > > > Key: KNOX-2319 > URL: https://issues.apache.org/jira/browse/KNOX-2319 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Minor > Fix For: 1.4.0 > > > Upgrade commons-compress 1.19 to 1.20 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (KNOX-2320) Upgrade xmlsec to 2.1.5
Kevin Risden created KNOX-2320: -- Summary: Upgrade xmlsec to 2.1.5 Key: KNOX-2320 URL: https://issues.apache.org/jira/browse/KNOX-2320 Project: Apache Knox Issue Type: Sub-task Reporter: Kevin Risden Assignee: Kevin Risden Fix For: 1.4.0 Upgrade xmlsec 2.1.4 to 2.1.5 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work started] (KNOX-2319) Upgrade commons-compress to 1.20
[ https://issues.apache.org/jira/browse/KNOX-2319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Work on KNOX-2319 started by Kevin Risden. -- > Upgrade commons-compress to 1.20 > > > Key: KNOX-2319 > URL: https://issues.apache.org/jira/browse/KNOX-2319 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Minor > Fix For: 1.4.0 > > > Upgrade commons-compress 1.19 to 1.20 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (KNOX-2319) Upgrade commons-compress to 1.20
Kevin Risden created KNOX-2319: -- Summary: Upgrade commons-compress to 1.20 Key: KNOX-2319 URL: https://issues.apache.org/jira/browse/KNOX-2319 Project: Apache Knox Issue Type: Sub-task Reporter: Kevin Risden Assignee: Kevin Risden Fix For: 1.4.0 Upgrade commons-compress to 1.20 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] risdenk commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
risdenk commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405701987 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: Does this logic need to be grouped somehow? Right now this is mixing && and ||? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418754&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418754 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 17:44 Start Date: 08/Apr/20 17:44 Worklog Time Spent: 10m Work Description: risdenk commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405701987 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -360,19 +370,43 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo // Record the new event query timestamp for this address/cluster setEventQueryTimestamp(address, clusterName, Instant.now()); -// Query the event log from CM for service/cluster restart events -List events = queryRestartEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), +// Query the event log from CM for service/cluster start events +List events = queryEvents(getApiClient(configCache.getDiscoveryConfig(address, clusterName)), clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + if(isRelevantEvent(event)) { +relevantEvents.add(new StartEvent(event)); + } +} + +return relevantEvents; + } + + @SuppressWarnings("unchecked") + private boolean isRelevantEvent(ApiEvent event) { +boolean rc = false; +String command = null; +String status = null; +List attributes = event.getAttributes(); +Map map = getAttributeMap(attributes); +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { Review comment: Does this logic need to be grouped somehow? Right now this is mixing && and ||? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418754) Time Spent: 2h 10m (was: 2h) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 2h 10m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Resolved] (KNOX-2268) Bump version dependencies March 2020
[ https://issues.apache.org/jira/browse/KNOX-2268?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kevin Risden resolved KNOX-2268. Resolution: Fixed > Bump version dependencies March 2020 > > > Key: KNOX-2268 > URL: https://issues.apache.org/jira/browse/KNOX-2268 > Project: Apache Knox > Issue Type: Improvement >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Major > Fix For: 1.4.0 > > > There are a dependencies that can be updated. Subtasks will be created for > each dependency so the change can be rolled back individually if necessary. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KNOX-2221) Upgrade shiro to 1.5.1
[ https://issues.apache.org/jira/browse/KNOX-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kevin Risden updated KNOX-2221: --- Parent Issue: KNOX-2318 (was: KNOX-2268) > Upgrade shiro to 1.5.1 > -- > > Key: KNOX-2221 > URL: https://issues.apache.org/jira/browse/KNOX-2221 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Major > Fix For: 1.5.0 > > Attachments: log.txt.gz > > Time Spent: 0.5h > Remaining Estimate: 0h > > *Note:* Can't upgrade to shiro 1.5.0 due to a bug in handling `/` only from > SHIRO-682. Will need to wait for a new version of Shiro to upgrade. > Upgrade shiro 1.4.2 to 1.5.1 > Shiro 1.5.0 release notes: > https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310950&version=12344991 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (KNOX-2318) Bump version dependencies April 2020
Kevin Risden created KNOX-2318: -- Summary: Bump version dependencies April 2020 Key: KNOX-2318 URL: https://issues.apache.org/jira/browse/KNOX-2318 Project: Apache Knox Issue Type: Improvement Reporter: Kevin Risden Assignee: Kevin Risden Fix For: 1.5.0 There are a dependencies that can be updated. Subtasks will be created for each dependency so the change can be rolled back individually if necessary. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KNOX-2283) Upgrade zookeeper to 3.6.0
[ https://issues.apache.org/jira/browse/KNOX-2283?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kevin Risden updated KNOX-2283: --- Parent Issue: KNOX-2318 (was: KNOX-2268) > Upgrade zookeeper to 3.6.0 > -- > > Key: KNOX-2283 > URL: https://issues.apache.org/jira/browse/KNOX-2283 > Project: Apache Knox > Issue Type: Sub-task >Reporter: Kevin Risden >Priority: Minor > Fix For: 1.5.0 > > > Upgrade zookeeper 3.5.7 to 3.6.0 > This requires CURATOR-558 due to some Zookeeper API changes. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KNOX-2268) Bump version dependencies March 2020
[ https://issues.apache.org/jira/browse/KNOX-2268?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kevin Risden updated KNOX-2268: --- Fix Version/s: (was: 1.5.0) 1.4.0 > Bump version dependencies March 2020 > > > Key: KNOX-2268 > URL: https://issues.apache.org/jira/browse/KNOX-2268 > Project: Apache Knox > Issue Type: Improvement >Reporter: Kevin Risden >Assignee: Kevin Risden >Priority: Major > Fix For: 1.4.0 > > > There are a dependencies that can be updated. Subtasks will be created for > each dependency so the change can be rolled back individually if necessary. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418652&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418652 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 16:11 Start Date: 08/Apr/20 16:11 Worklog Time Spent: 10m Work Description: lmccay commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405643342 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: I think the latest revision will make the above more clear, Kevin. Thanks for the review! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418652) Time Spent: 2h (was: 1h 50m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 2h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405643342 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: I think the latest revision will make the above more clear, Kevin. Thanks for the review! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [knox] lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405642911 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { Review comment: Latest revision addresses the "restart" terminology and cleans up the method as you suggested offline. Thanks for the review! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418649&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418649 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 16:10 Start Date: 08/Apr/20 16:10 Worklog Time Spent: 10m Work Description: lmccay commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405642911 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { Review comment: Latest revision addresses the "restart" terminology and cleans up the method as you suggested offline. Thanks for the review! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418649) Time Spent: 1h 50m (was: 1h 40m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 1h 50m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418612&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418612 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 15:13 Start Date: 08/Apr/20 15:13 Worklog Time Spent: 10m Work Description: lmccay commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405601417 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { Review comment: I didn't see any point in changing it to reflect Start and Restart the are both "start" events. We can change to Start which is even a semantically meaningless change. I wanted to keep the map external rather than create it inside that method in case we need to add an additional use of the map later. We can move it in and refactor later if needed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418612) Time Spent: 1h 40m (was: 1.5h) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 1h 40m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405601417 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { Review comment: I didn't see any point in changing it to reflect Start and Restart the are both "start" events. We can change to Start which is even a semantically meaningless change. I wanted to keep the map external rather than create it inside that method in case we need to add an additional use of the map later. We can move it in and refactor later if needed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418604&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418604 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 15:11 Start Date: 08/Apr/20 15:11 Worklog Time Spent: 10m Work Description: lmccay commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405598727 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: Yes, the map is redone for each event purposely. It is only a map of the attributes for the given event. The order of the events hasn't changed as a result of this patch and is driven by the REST API response and has nothing to do with the creation of this map. It works as intended. Regarding your question SERVICEA and SERVICEB - both events are preserved. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418604) Time Spent: 1.5h (was: 1h 20m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 1.5h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405598727 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: Yes, the map is redone for each event purposely. It is only a map of the attributes for the given event. The order of the events hasn't changed as a result of this patch and is driven by the REST API response and has nothing to do with the creation of this map. It works as intended. Regarding your question SERVICEA and SERVICEB - both events are preserved. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418600&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418600 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 15:10 Start Date: 08/Apr/20 15:10 Worklog Time Spent: 10m Work Description: lmccay commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405598727 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: Yes, the map is redone for each event purposely. It is only a map of the attributes for the given event. The order of the events hasn't changed as a result of this patch and is driven by the REST API response and has nothing to do with the creation of this map. It works as intended. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418600) Time Spent: 1h 20m (was: 1h 10m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 1h 20m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405598727 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: Yes, the map is redone for each event purposely. It is only a map of the attributes for the given event. The order of the events hasn't changed as a result of this patch and is driven by the REST API response and has nothing to do with the creation of this map. It works as intended. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418598&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418598 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 15:05 Start Date: 08/Apr/20 15:05 Worklog Time Spent: 10m Work Description: pzampino commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405595229 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { Review comment: Since the map is derived from the ApiEvent, addIfRelevantEvent should not need the attribute Map passed in as a param. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418598) Time Spent: 1h 10m (was: 1h) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 1h 10m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405595229 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { Review comment: Since the map is derived from the ApiEvent, addIfRelevantEvent should not need the attribute Map passed in as a param. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418595&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418595 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 15:01 Start Date: 08/Apr/20 15:01 Worklog Time Spent: 10m Work Description: pzampino commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405592686 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: The map is associated with a specific ApiEvent, so I'm not sure that the map is the concern. We may want to sort the ApiEvent collection by _timeOccurred_ to avoid issues associated with what you're describing. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418595) Time Spent: 1h (was: 50m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 1h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405592686 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: The map is associated with a specific ApiEvent, so I'm not sure that the map is the concern. We may want to sort the ApiEvent collection by _timeOccurred_ to avoid issues associated with what you're describing. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418592&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418592 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 14:51 Start Date: 08/Apr/20 14:51 Worklog Time Spent: 10m Work Description: pzampino commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405529175 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { Review comment: These aren't only restart events at this point, correct? Perhaps, RestartEvent should be renamed to ConfigChangeEvent or something more appropriate. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418592) Time Spent: 50m (was: 40m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 50m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
pzampino commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405529175 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { Review comment: These aren't only restart events at this point, correct? Perhaps, RestartEvent should be renamed to ConfigChangeEvent or something more appropriate. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418577&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418577 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 14:44 Start Date: 08/Apr/20 14:44 Worklog Time Spent: 10m Work Description: risdenk commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405577731 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); Review comment: Hmm I guess I am now more concerned with the logic here potentially then. I'll comment on the specific section below. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418577) Time Spent: 40m (was: 0.5h) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 40m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418578&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418578 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 14:44 Start Date: 08/Apr/20 14:44 Worklog Time Spent: 10m Work Description: risdenk commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405579448 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: This will overwrite in the map correct? The attribute get name - is that unique? I think this is by COMMAND or COMMAND_STATUS based on the logic for eventually checking the map. If there are duplicate commands won't they get overwritten in the map? So its last write wins? If you do a Start of SERVICEA and then Start of SERVICEB - will you see both events or just collapse it down to one? Another example is doing start of same service twice with different results. start success then (implicit stop) then start failure. Will that get picked up? I think the map is totally breaking the uniqueness and ordering of the events. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418578) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 40m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] risdenk commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
risdenk commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405577731 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); Review comment: Hmm I guess I am now more concerned with the logic here potentially then. I'll comment on the specific section below. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
risdenk commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405579448 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); + addIfRelevantEvent(restartEvents, event, map); } return restartEvents; } + @SuppressWarnings("unchecked") + private void addIfRelevantEvent(List restartEvents, ApiEvent event, Map map) { +String command = null; +String status = null; +command = (String) ((List) map.get(COMMAND)).get(0); +status = (String) ((List) map.get(COMMAND_STATUS)).get(0); +if (START_COMMAND.equals(command) || RESTART_COMMAND.equals(command) && +SUCCEEDED_STATUS.equals(status) || STARTED_STATUS.equals(status)) { + restartEvents.add(new RestartEvent(event)); +} + } + + private Map getAttributeMap(List attributes) { +Map map = new HashMap<>(); +attributes.forEach(attr -> { map.put(attr.getName(), attr.getValues());}); Review comment: This will overwrite in the map correct? The attribute get name - is that unique? I think this is by COMMAND or COMMAND_STATUS based on the logic for eventually checking the map. If there are duplicate commands won't they get overwritten in the map? So its last write wins? If you do a Start of SERVICEA and then Start of SERVICEB - will you see both events or just collapse it down to one? Another example is doing start of same service twice with different results. start success then (implicit stop) then start failure. Will that get picked up? I think the map is totally breaking the uniqueness and ordering of the events. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418571&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418571 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 14:34 Start Date: 08/Apr/20 14:34 Worklog Time Spent: 10m Work Description: lmccay commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405572741 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); Review comment: That is what I had before and it is much more complicated as I wanted to avoid creating a map. Note that we are checking two correlated attributes. The command issued and its status. Since you don't know what order they are in you have to squirrel them away then check whether you have both then add. If we end up having to add more events that would be even messier to do. Now, we can get the map upfront and do whatever processing of it afterward much more cleanly. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418571) Time Spent: 0.5h (was: 20m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 0.5h > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
lmccay commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405572741 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); Review comment: That is what I had before and it is much more complicated as I wanted to avoid creating a map. Note that we are checking two correlated attributes. The command issued and its status. Since you don't know what order they are in you have to squirrel them away then check whether you have both then add. If we end up having to add more events that would be even messier to do. Now, we can get the map upfront and do whatever processing of it afterward much more cleanly. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (KNOX-2304) CM discovery - cluster config monitor needs to be aware of all relevant CM event types
[ https://issues.apache.org/jira/browse/KNOX-2304?focusedWorklogId=418518&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-418518 ] ASF GitHub Bot logged work on KNOX-2304: Author: ASF GitHub Bot Created on: 08/Apr/20 13:28 Start Date: 08/Apr/20 13:28 Worklog Time Spent: 10m Work Description: risdenk commented on pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405524259 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); Review comment: Why not loop through the ApiEventAttribute list directly? There doesn't seem to be any value in converting to a Map. You break the map apart in the same way you would have to interrogate the ApiEventAttribute anyway. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 418518) Time Spent: 20m (was: 10m) > CM discovery - cluster config monitor needs to be aware of all relevant CM > event types > -- > > Key: KNOX-2304 > URL: https://issues.apache.org/jira/browse/KNOX-2304 > Project: Apache Knox > Issue Type: Bug > Components: cm-discovery >Affects Versions: 1.4.0 >Reporter: Philip Zampino >Assignee: Philip Zampino >Priority: Major > Fix For: 1.4.0 > > Time Spent: 20m > Remaining Estimate: 0h > > The CM cluster config monitor currently only polls for restart events. There > are other event types (e.g., start) that should trigger the configuration > analysis and subsequently trigger re-discovery. > Need to determine the full set of event types that apply, and update the > polling query. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [knox] risdenk commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of …
risdenk commented on a change in pull request #307: KNOX-2304 - CM discovery cluster config monitor needs to be aware of … URL: https://github.com/apache/knox/pull/307#discussion_r405524259 ## File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java ## @@ -365,12 +375,32 @@ private DiscoveryApiClient getApiClient(final ServiceDiscoveryConfig discoveryCo clusterName, lastTimestamp); for (ApiEvent event : events) { - restartEvents.add(new RestartEvent(event)); + List attributes = event.getAttributes(); + Map map = getAttributeMap(attributes); Review comment: Why not loop through the ApiEventAttribute list directly? There doesn't seem to be any value in converting to a Map. You break the map apart in the same way you would have to interrogate the ApiEventAttribute anyway. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services