Skip to content

Commit

Permalink
Fixes #6005: sort the reports execution not by date, but by insertion
Browse files Browse the repository at this point in the history
  • Loading branch information
ncharles committed Dec 16, 2014
1 parent 59d94eb commit 20d78e0
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 32 deletions.
@@ -0,0 +1,58 @@
/*
*************************************************************************************
* Copyright 2014 Normation SAS
*************************************************************************************
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* In accordance with the terms of section 7 (7. Additional Terms.) of
* the GNU Affero GPL v3, the copyright holders add the following
* Additional permissions:
* Notwithstanding to the terms of section 5 (5. Conveying Modified Source
* Versions) and 6 (6. Conveying Non-Source Forms.) of the GNU Affero GPL v3
* licence, when you create a Related Module, this Related Module is
* not considered as a part of the work and may be distributed under the
* license agreement of your choice.
* A "Related Module" means a set of sources files including their
* documentation that, without modification of the Source Code, enables
* supplementary functions or services in addition to those offered by
* the Software.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/agpl.html>.
*
*************************************************************************************
*/


-- add the columns needed to store the report insertion id:
-- in reportExectution, "insertionId"
ALTER TABLE ReportsExecution ADD COLUMN insertionId bigint;

-- Add the index on this entry
CREATE INDEX reportsexecution_insertionid_idx ON ReportsExecution (insertionId);

-- Do the migration script
-- Add the id from the ruddersysevents when available, but only for the last run of each node

UPDATE ReportsExecution SET insertionId = S.id
FROM
(
SELECT T.nodeid, T.executiontimestamp, min(id) AS id FROM
(
SELECT DISTINCT nodeid, max(date) AS executiontimestamp FROM ReportsExecution GROUP BY nodeid
) AS T
LEFT JOIN ruddersysevents AS R
ON T.nodeid = R.nodeid AND T.executiontimestamp = R.executiontimestamp
GROUP BY T.nodeid, T.executiontimestamp
) AS S WHERE testreport.date = S.executiontimestamp AND testreport.nodeid=S.nodeid;


2 changes: 2 additions & 0 deletions rudder-core/src/main/resources/reportsSchema.sql
Expand Up @@ -117,10 +117,12 @@ CREATE TABLE ReportsExecution (
, date timestamp with time zone NOT NULL
, complete boolean NOT NULL
, nodeConfigId text
, insertionId bigint
, PRIMARY KEY(nodeId, date)
);

CREATE INDEX reportsexecution_date_idx ON ReportsExecution (date);
CREATE INDEX reportsexecution_insertionid_idx ON ReportsExecution (insertionId);

/*
*************************************************************************************
Expand Down
Expand Up @@ -51,4 +51,5 @@ final case class AgentRun (
agentRunId : AgentRunId
, nodeConfigVersion: Option[NodeConfigId]
, isCompleted : Boolean
, insertionId : Long
)
Expand Up @@ -46,6 +46,8 @@ trait RoReportsExecutionRepository {

/**
* Find the last execution of nodes, whatever is its state.
* Last execution is defined as "the last executions that have been inserted in the database",
* and do not rely on date (which can change too often)
*/
def getNodesLastRun(nodeIds: Set[NodeId]): Box[Map[NodeId, Option[AgentRun]]]
}
Expand Down
Expand Up @@ -68,7 +68,8 @@ case class RoReportsExecutionJdbcRepository (
val nodeId = NodeId(rs.getString("nodeid"))
val id = AgentRunId(nodeId, new DateTime(rs.getTimestamp("date").getTime))
val configId = Option(rs.getString("nodeconfigid")).map(NodeConfigId(_))
(nodeId, AgentRun(id, configId, rs.getBoolean("complete")))
val insertionId = rs.getLong("insertionid")
(nodeId, AgentRun(id, configId, rs.getBoolean("complete"), insertionId))
}
}

Expand All @@ -78,10 +79,10 @@ case class RoReportsExecutionJdbcRepository (
import pgInClause.in
val query =
s"""SELECT DISTINCT ON (nodeid)
| nodeid, date, nodeconfigid, complete
| nodeid, date, nodeconfigid, complete, insertionid
|FROM reportsexecution
|WHERE ${in("nodeid", nodeIds.map(_.value))}
|ORDER BY nodeid, date DESC""".stripMargin
|ORDER BY nodeid, insertionId DESC""".stripMargin

val errorMSg = s"Error when trying to get report executions for nodes with Id '${nodeIds.map( _.value).mkString(",")}'"

Expand Down Expand Up @@ -172,11 +173,11 @@ object ExecutionRepositoryUtils {
}

implicit def toDB (execution : AgentRun) : DBAgentRun = {
DBAgentRun(execution.agentRunId.nodeId.value, execution.agentRunId.date, execution.nodeConfigVersion.map(_.value), execution.isCompleted)
DBAgentRun(execution.agentRunId.nodeId.value, execution.agentRunId.date, execution.nodeConfigVersion.map(_.value), execution.isCompleted, execution.insertionId)
}

implicit def fromDB (execution : DBAgentRun) : AgentRun = {
AgentRun(AgentRunId(NodeId(execution.nodeId), new DateTime(execution.date)), execution.nodeConfigId.map(NodeConfigId(_)), execution.isCompleted)
AgentRun(AgentRunId(NodeId(execution.nodeId), new DateTime(execution.date)), execution.nodeConfigId.map(NodeConfigId(_)), execution.isCompleted, execution.insertionId)
}
}

Expand All @@ -186,10 +187,11 @@ object Executions extends Schema {
}

case class DBAgentRun (
@Column("nodeid") nodeId : String
, @Column("date") date : Timestamp
@Column("nodeid") nodeId : String
, @Column("date") date : Timestamp
, @Column("nodeconfigid") nodeConfigId : Option[String]
, @Column("complete") isCompleted: Boolean
, @Column("complete") isCompleted : Boolean
, @Column("insertionid") insertionId: Long
) extends KeyedEntity[CompositeKey2[String,Timestamp]] {

def id = compositeKey(nodeId,date)
Expand Down
Expand Up @@ -407,9 +407,9 @@ class ReportsJdbcRepository(jdbcTemplate : JdbcTemplate) extends ReportsReposito
def getRuns(fromId: Long, toId: Long): Box[Seq[AgentRun]] = {
import java.lang.{Long => jLong}
val getRunsQuery = """select distinct
| T.nodeid, T.executiontimestamp, coalesce(C.iscomplete, false) as complete, coalesce(C.msg, '') as nodeconfigid
| T.nodeid, T.executiontimestamp, coalesce(C.iscomplete, false) as complete, coalesce(C.msg, '') as nodeconfigid, T.insertionid
|from
| (select distinct nodeid, executiontimestamp from ruddersysevents where id > ? and id <= ?) as T
| (select nodeid, executiontimestamp, min(id) as insertionid from ruddersysevents where id > ? and id <= ? group by nodeid, executiontimestamp) as T
|left join
| (select
| true as isComplete, nodeid, executiontimestamp, msg
Expand Down Expand Up @@ -538,6 +538,7 @@ object ReportsExecutionMapper extends RowMapper[AgentRun] {
}
}
, rs.getBoolean("complete")
, rs.getLong("insertionid")
)
}
}
Expand Up @@ -284,10 +284,10 @@ object ExecutionBatch extends Loggable {
}
}

case (Some(AgentRun(AgentRunId(_, t), None, _)), None) =>
case (Some(AgentRun(AgentRunId(_, t), None, _, _)), None) =>
VersionNotInitialized(t)

case (Some(AgentRun(AgentRunId(_, t), None, _)), Some(configs)) =>
case (Some(AgentRun(AgentRunId(_, t), None, _, _)), Some(configs)) =>
if(configs.isEmpty) {
VersionNotInitialized(t)
} else {
Expand Down Expand Up @@ -322,15 +322,15 @@ object ExecutionBatch extends Loggable {
}
}

case (Some(AgentRun(AgentRunId(_, t), Some(rv), _)), None) =>
case (Some(AgentRun(AgentRunId(_, t), Some(rv), _, _)), None) =>
//that seems to indicate a bug, the node should not be able to have
//a version without having any information in the table.
//migration from an other Rudder ? table deleted ?
logger.debug(s"Agent for node '${nodeId.value}' sent the node config identifier '${rv.value}' which is not known by Rudder. Policy for that node should be regenerated" )
//in all case, we want to act as if the node didn't had any config id.
VersionNotInitialized(t)

case (Some(AgentRun(AgentRunId(_, t), Some(rv), _)), Some(configs)) =>
case (Some(AgentRun(AgentRunId(_, t), Some(rv), _, _)), Some(configs)) =>
if(configs.isEmpty) {
//error: we have a MISSING config id. Contrary to the case where any config id is missing
//for the node, here we have a BAD id.
Expand Down
Expand Up @@ -81,8 +81,8 @@ class AgentRunsTest extends DBCommon {

"Execution repo" should {
val runs = Seq(
AgentRun(AgentRunId(n1, runMinus2), Some(NodeConfigId("nodeConfig_n1_v1")), true)
, AgentRun(AgentRunId(n1, runMinus1), Some(NodeConfigId("nodeConfig_n1_v1")), false)
AgentRun(AgentRunId(n1, runMinus2), Some(NodeConfigId("nodeConfig_n1_v1")), true, 12)
, AgentRun(AgentRunId(n1, runMinus1), Some(NodeConfigId("nodeConfig_n1_v1")), false, 42)
)

"correctly insert" in {
Expand All @@ -105,9 +105,9 @@ class AgentRunsTest extends DBCommon {


val initRuns = Seq(
AgentRun(AgentRunId(n1, runMinus2.minusMinutes(5)), Some(NodeConfigId("nodeConfig_n1_v1")), true)
, AgentRun(AgentRunId(n1, runMinus2), Some(NodeConfigId("nodeConfig_n1_v1")), true)
, AgentRun(AgentRunId(n1, runMinus1), Some(NodeConfigId("nodeConfig_n1_v1")), false)
AgentRun(AgentRunId(n1, runMinus2.minusMinutes(5)), Some(NodeConfigId("nodeConfig_n1_v1")), true, 12)
, AgentRun(AgentRunId(n1, runMinus2), Some(NodeConfigId("nodeConfig_n1_v1")), true, 42)
, AgentRun(AgentRunId(n1, runMinus1), Some(NodeConfigId("nodeConfig_n1_v1")), false, 64)
)

/*
Expand All @@ -127,8 +127,8 @@ class AgentRunsTest extends DBCommon {
initRuns(0).copy(isCompleted = false) //not updated
, initRuns(1).copy(nodeConfigVersion = Some(NodeConfigId("nodeConfig_n1_v2")))
, initRuns(2).copy(isCompleted = true)
, AgentRun(AgentRunId(n1, runMinus2.minusMinutes(10)), Some(NodeConfigId("nodeConfig_n1_v1")), true)
, AgentRun(AgentRunId(n1, runMinus1.plusMinutes(5)), Some(NodeConfigId("nodeConfig_n1_v1")), false)
, AgentRun(AgentRunId(n1, runMinus2.minusMinutes(10)), Some(NodeConfigId("nodeConfig_n1_v1")), true, 12)
, AgentRun(AgentRunId(n1, runMinus1.plusMinutes(5)), Some(NodeConfigId("nodeConfig_n1_v1")), false, 42)
)

//only the first one should not be modified
Expand Down
Expand Up @@ -179,11 +179,11 @@ class ReportsTest extends DBCommon {
"get reports" in {
val res = repostsRepo.getReportsfromId(0, DateTime.now().plusDays(1)).openOrThrowException("Test failed")
val expected = Seq(
AgentRun(AgentRunId(NodeId("n1"),run2),None,false)
, AgentRun(AgentRunId(NodeId("n2"),run1),Some(NodeConfigId("n2_run1")),true)
, AgentRun(AgentRunId(NodeId("n1"),run1),Some(NodeConfigId("n1_run1")),true)
, AgentRun(AgentRunId(NodeId("n1"),run3),None,false)
, AgentRun(AgentRunId(NodeId("n0"),run1),None,true)
AgentRun(AgentRunId(NodeId("n1"),run2),None,false, 116)
, AgentRun(AgentRunId(NodeId("n2"),run1),Some(NodeConfigId("n2_run1")),true, 119)
, AgentRun(AgentRunId(NodeId("n1"),run1),Some(NodeConfigId("n1_run1")),true, 110)
, AgentRun(AgentRunId(NodeId("n1"),run3),None,false, 118)
, AgentRun(AgentRunId(NodeId("n0"),run1),None,true, 109)
)

res._1 must contain(exactly(expected:_*))
Expand Down
Expand Up @@ -230,10 +230,10 @@ class ReportingServiceTest extends DBCommon {

res must beEqualTo(agentRuns(
("n0" -> None )
, ("n1" -> Some(( run1, None , true )))
, ("n2" -> Some(( run1, Some("n2_t1"), true )))
, ("n3" -> Some(( run2, None , true )))
, ("n4" -> Some(( run2, Some("n4_t2"), true )))
, ("n1" -> Some(( run1, None , true, 106 )))
, ("n2" -> Some(( run1, Some("n2_t1"), true, 102 )))
, ("n3" -> Some(( run2, None , true, 126 )))
, ("n4" -> Some(( run2, Some("n4_t2"), true, 116 )))
))

}
Expand Down Expand Up @@ -783,9 +783,9 @@ class ReportingServiceTest extends DBCommon {

implicit def toRuleId(id: String): RuleId = RuleId(id)

implicit def agentRuns(runs:(String, Option[(DateTime, Option[String], Boolean)])*): Map[NodeId, Option[AgentRun]] = {
implicit def agentRuns(runs:(String, Option[(DateTime, Option[String], Boolean, Long)])*): Map[NodeId, Option[AgentRun]] = {
runs.map { case (id, opt) =>
NodeId(id) -> opt.map(e => AgentRun(AgentRunId(NodeId(id), e._1), e._2.map(NodeConfigId(_)), e._3))
NodeId(id) -> opt.map(e => AgentRun(AgentRunId(NodeId(id), e._1), e._2.map(NodeConfigId(_)), e._3, e._4))
}.toMap
}
}

0 comments on commit 20d78e0

Please sign in to comment.