Skip to content

OCPBUGS-179: fix conditional edge for type always #730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PratikMahajan merged 2 commits into from
Aug 17, 2022
Merged

OCPBUGS-179: fix conditional edge for type always #730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9db3fe
fix conditional edge for type always
PratikMahajan Aug 17, 2022
297f59c
add location flag for oc binary download
PratikMahajan Aug 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions cincinnati/src/conditional_edges.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,16 @@ pub struct ConditionalUpdateRisk {
#[serde(default)]
pub struct ClusterCondition {
#[serde(rename = "type")]
condition_type: String,
promql: PromQLClusterCondition,
pub condition_type: String,
#[serde(skip_serializing_if = "PromQLClusterCondition::is_empty")]
pub promql: PromQLClusterCondition,
}

/// Contains the PromQL string
#[derive(Debug, Serialize, Deserialize, SmartDefault, Clone, Eq, PartialEq, Hash)]
#[serde(default)]
struct PromQLClusterCondition {
promql: String,
pub struct PromQLClusterCondition {
pub promql: String,
}

impl ConditionalEdge {
Expand All @@ -51,3 +52,10 @@ impl ConditionalEdge {
&mut self.edges
}
}

impl PromQLClusterCondition {
/// returns true if there is no PromQL condition to serialize.
pub fn is_empty(&self) -> bool {
self.promql.is_empty()
}
}
60 changes: 53 additions & 7 deletions cincinnati/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,12 +818,49 @@ pub mod testing {
use super::*;
use crate::plugins::internal::versioned_graph::VersionedGraph;

pub fn generate_graph(include_conditional_edge: bool) -> Graph {
pub fn generate_graph(include_conditional_edge: bool, include_always_condition: bool) -> Graph {
let mut graph = Graph::default();

if include_conditional_edge {
let mut ce = ConditionalEdge {
edge_regex: ConditionalUpdateEdge {
from: "1[.]0[.].*".to_string(),
to: "2.0.0".to_string(),
},
edges: vec![ConditionalUpdateEdge {
from: "1.0.0".to_string(),
to: "2.0.0".to_string(),
}],
risks: vec![ConditionalUpdateRisk {
url: "https://bug.example.com/show_bug.cgi?id=example".to_string(),
name: "BrokenUpdates".to_string(),
message: "Updates are broken for this provider".to_string(),
matching_rules: vec![ClusterCondition {
condition_type: "PromQL".to_string(),
promql: PromQLClusterCondition {
Copy link
Copy Markdown
Member

@wking wking Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we leave this unpopulated, what happens?

Copy link
Copy Markdown
Contributor Author

@PratikMahajan PratikMahajan Aug 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using Default:default() for Always condition

promql: "cluster_infrastructure_provider{type=\"CloudProvider\"}"
.to_string(),
},
}],
}],
};
if include_always_condition {
ce.risks = vec![ConditionalUpdateRisk {
url: "https://bug.example.com/show_bug.cgi?id=example".to_string(),
name: "AllBrokenUpdates".to_string(),
message: "All Updates are broken".to_string(),
matching_rules: vec![ClusterCondition {
condition_type: "Always".to_string(),
promql: Default::default(),
}],
}]
}
graph.conditional_edges = Some(vec![ce]);
}
if !include_conditional_edge {
graph.conditional_edges = None;
}

let v1 = graph.dag.add_node(Release::Concrete(ConcreteRelease {
version: String::from("1.0.0"),
payload: String::from("image/1.0.0"),
Expand Down Expand Up @@ -1160,7 +1197,7 @@ mod tests {

#[test]
fn serialize_graph() {
let graph = generate_graph(false);
let graph = generate_graph(false, false);
assert_eq!(
serde_json::to_string(&graph).unwrap(),
r#"{"nodes":[{"version":"1.0.0","payload":"image/1.0.0","metadata":{}},{"version":"2.0.0","payload":"image/2.0.0","metadata":{}},{"version":"3.0.0","payload":"image/3.0.0","metadata":{}}],"edges":[[0,1],[1,2],[0,2]]}"#
Expand All @@ -1169,10 +1206,19 @@ mod tests {

#[test]
fn serialize_graph_with_conditional_edges() {
let graph = generate_graph(true);
let graph = generate_graph(true, false);
assert_eq!(
serde_json::to_string(&graph).unwrap(),
r#"{"nodes":[{"version":"1.0.0","payload":"image/1.0.0","metadata":{}},{"version":"2.0.0","payload":"image/2.0.0","metadata":{}},{"version":"3.0.0","payload":"image/3.0.0","metadata":{}}],"edges":[[0,1],[1,2],[0,2]],"conditionalEdges":[{"edges":[{"from":"1.0.0","to":"2.0.0"}],"risks":[{"url":"https://bug.example.com/show_bug.cgi?id=example","name":"BrokenUpdates","message":"Updates are broken for this provider","matchingRules":[{"type":"PromQL","promql":{"promql":"cluster_infrastructure_provider{type=\"CloudProvider\"}"}}]}]}]}"#
);
}

#[test]
fn serialize_graph_with_conditional_edges_always() {
let graph = generate_graph(true, true);
assert_eq!(
serde_json::to_string(&graph).unwrap(),
r#"{"nodes":[{"version":"1.0.0","payload":"image/1.0.0","metadata":{}},{"version":"2.0.0","payload":"image/2.0.0","metadata":{}},{"version":"3.0.0","payload":"image/3.0.0","metadata":{}}],"edges":[[0,1],[1,2],[0,2]],"conditionalEdges":[]}"#
r#"{"nodes":[{"version":"1.0.0","payload":"image/1.0.0","metadata":{}},{"version":"2.0.0","payload":"image/2.0.0","metadata":{}},{"version":"3.0.0","payload":"image/3.0.0","metadata":{}}],"edges":[[0,1],[1,2],[0,2]],"conditionalEdges":[{"edges":[{"from":"1.0.0","to":"2.0.0"}],"risks":[{"url":"https://bug.example.com/show_bug.cgi?id=example","name":"AllBrokenUpdates","message":"All Updates are broken","matchingRules":[{"type":"Always"}]}]}]}"#
);
}

Expand Down Expand Up @@ -1226,7 +1272,7 @@ mod tests {

#[test]
fn test_graph_eq_true_for_equal_graphs() {
assert_eq!(generate_graph(false), generate_graph(false))
assert_eq!(generate_graph(false, false), generate_graph(false, false))
}

#[test]
Expand Down Expand Up @@ -1314,10 +1360,10 @@ mod tests {

#[test]
fn roundtrip_conversion_from_graph_via_plugin_interface() {
let graph_plugin_interface: plugins::interface::Graph = generate_graph(false).into();
let graph_plugin_interface: plugins::interface::Graph = generate_graph(false, false).into();
let graph_native_converted: Graph = graph_plugin_interface.into();

assert_eq!(generate_graph(false), graph_native_converted);
assert_eq!(generate_graph(false, false), graph_native_converted);
}

fn get_test_metadata_fn_mut(key_prefix: &str, key_suffix: &str) -> TestMetadata {
Expand Down
4 changes: 2 additions & 2 deletions cincinnati/src/plugins/external/web.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ mod tests {
});

let input_internal = InternalIO {
graph: generate_graph(false),
graph: generate_graph(false, false),
parameters: [("hello".to_string(), "plugin".to_string())]
.iter()
.cloned()
Expand Down Expand Up @@ -107,7 +107,7 @@ mod tests {
});

let input_internal = InternalIO {
graph: generate_graph(false),
graph: generate_graph(false, false),
parameters: [("hello".to_string(), "plugin".to_string())]
.iter()
.cloned()
Expand Down
10 changes: 5 additions & 5 deletions cincinnati/src/plugins/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ mod tests {

#[test]
fn convert_roundtrip_internalio_externalio() {
let graph = generate_graph(false);
let graph = generate_graph(false, false);
let input_internal = InternalIO {
graph,
parameters: [("hello".to_string(), "plugin".to_string())]
Expand Down Expand Up @@ -568,15 +568,15 @@ mod tests {
}

let initial_internalio = InternalIO {
graph: generate_graph(false),
graph: generate_graph(false, false),
parameters: [("hello".to_string(), "plugin".to_string())]
.iter()
.cloned()
.collect(),
};

let expected_internalio = InternalIO {
graph: generate_graph(false),
graph: generate_graph(false, false),
parameters: [
("hello".to_string(), "plugin".to_string()),
("COUNTER".to_string(), "1".to_string()),
Expand Down Expand Up @@ -613,7 +613,7 @@ mod tests {
}

let initial_internalio = InternalIO {
graph: generate_graph(false),
graph: generate_graph(false, false),
parameters: [("hello".to_string(), "plugin".to_string())]
.iter()
.cloned()
Expand All @@ -624,7 +624,7 @@ mod tests {

for i in 0..runs {
let expected_internalio = InternalIO {
graph: generate_graph(false),
graph: generate_graph(false, false),
parameters: [
("hello".to_string(), "plugin".to_string()),
("COUNTER".to_string(), format!("{}", i + 1)),
Expand Down
2 changes: 1 addition & 1 deletion dist/Dockerfile.e2e-ubi8/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ WORKDIR "${HOME}/cincinnati"

# Get oc CLI
RUN mkdir -p ${HOME}/bin && \
curl https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/openshift-client-linux.tar.gz 2>/dev/null | tar xzf - -C "${HOME}/bin/" oc
curl -L https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/openshift-client-linux.tar.gz 2>/dev/null | tar xzf - -C "${HOME}/bin/" oc
ENV PATH="${PATH}:${HOME}/bin"

COPY --from=rust_builder /opt/cincinnati/bin/e2e /usr/bin/cincinnati-e2e-test
Expand Down
2 changes: 1 addition & 1 deletion dist/Dockerfile.e2e/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ WORKDIR "${HOME}/cincinnati"

# Get oc CLI
RUN mkdir -p ${HOME}/bin && \
curl https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/openshift-client-linux.tar.gz 2>/dev/null | tar xzf - -C "${HOME}/bin/" oc
curl -L https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/openshift-client-linux.tar.gz 2>/dev/null | tar xzf - -C "${HOME}/bin/" oc
ENV PATH="${PATH}:${HOME}/bin"

# Install container tools
Expand Down