Skip to content
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.

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

Updates to trace config format as well as bugfix in sim runner #143

Merged
merged 3 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
url = "2.4.1"

# test dependencies
assertables = "7.0.1"
assertables = "8.18.0"
http = "0.2.9"
httpmock = "0.6.8"
hyper = "0.14.27"
Expand Down
2 changes: 2 additions & 0 deletions k8s/kustomize/prod/sk-tracer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ data:
trackedObjects:
apps/v1.Deployment:
podSpecTemplatePath: /spec/template
v1.ServiceAccount: {}
v1.ConfigMap: {}
---
apiVersion: apps/v1
kind: Deployment
Expand Down
2 changes: 2 additions & 0 deletions k8s/sk_tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
trackedObjects:
apps/v1.Deployment:
podSpecTemplatePath: /spec/template
v1.ServiceAccount: {}
v1.ConfigMap: {}
"""
CONFIGMAP_NAME = "tracer-config"

Expand Down
3 changes: 2 additions & 1 deletion sk-cli/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ pub struct Args {
long,
long_help = "namespaces to exclude from the trace",
value_delimiter = ',',
default_value = "cert-manager,kube-system,local-path-storage,monitoring,simkube"
default_value = "cert-manager,default,kube-public,kube-node-lease,kube-system,\
local-path-storage,monitoring,simkube"
)]
pub excluded_namespaces: Vec<String>,

Expand Down
71 changes: 60 additions & 11 deletions sk-core/src/k8s/gvk.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::fmt;
use std::ops::Deref;

Expand All @@ -20,6 +21,8 @@ use crate::prelude::*;
// custom serialization methods. We also add some handy helper/conversion functions.
//
// Specifically for serialization/deserialization, we convert to the format "group/version.kind".
// (unless the group is "core", and then we serialize to "version.kind", but can deserialize from
// either "version.kind" or "/version.kind" for backwards compatibility)
#[derive(Clone, Debug, Hash, Eq, PartialEq)]
pub struct GVK(GroupVersionKind);

Expand Down Expand Up @@ -62,7 +65,12 @@ impl Serialize for GVK {
where
S: Serializer,
{
let skey = format!("{}/{}.{}", self.0.group, self.0.version, self.0.kind);
let mut group = Cow::from(&self.0.group);
if !group.is_empty() {
group.to_mut().push('/');
}

let skey = format!("{group}{}.{}", self.0.version, self.0.kind);
serializer.serialize_str(&skey)
}
}
Expand All @@ -81,16 +89,18 @@ impl<'de> de::Visitor<'de> for GVKVisitor {
E: de::Error,
{
let p1: Vec<_> = value.split('/').collect();
if p1.len() != 2 {
return Err(E::custom(format!("invalid format for gvk: {value}")));
}
let p2: Vec<_> = p1[1].split('.').collect();
if p2.len() != 2 {
return Err(E::custom(format!("invalid format for gvk: {value}")));
}

let parts = [p1[0], p2[0], p2[1]];
Ok(GVK(GroupVersionKind::gvk(parts[0], parts[1], parts[2])))
let (group, rest) = match p1.len() {
2 => (p1[0], p1[1]),
1 => ("", p1[0]),
_ => return Err(E::custom(format!("invalid format for gvk: {value}"))),
};
let p2: Vec<_> = rest.split('.').collect();
let (version, kind) = match p2.len() {
2 => (p2[0], p2[1]),
_ => return Err(E::custom(format!("invalid format for gvk: {value}"))),
};

Ok(GVK(GroupVersionKind::gvk(group, version, kind)))
}
}

Expand All @@ -102,3 +112,42 @@ impl<'de> Deserialize<'de> for GVK {
deserializer.deserialize_str(GVKVisitor)
}
}

#[cfg(test)]
mod test {
use assertables::*;
use rstest::*;
use serde::de::value::{
Error as SerdeError,
StrDeserializer,
};
use serde::de::IntoDeserializer;

use super::*;

#[rstest]
fn test_serialize() {
// I had to think about this for a minute, but strings in JSON have to include quotes,
// which is why they're escaped out here.
assert_eq!(serde_json::to_string(&GVK::new("foo", "v1", "bar")).unwrap(), "\"foo/v1.bar\"");
assert_eq!(serde_json::to_string(&GVK::new("", "v1", "bar")).unwrap(), "\"v1.bar\"");
}

#[rstest]
fn test_deserialize() {
let d1: StrDeserializer<SerdeError> = "foo/v1.bar".into_deserializer();
assert_eq!(GVK::deserialize(d1).unwrap(), GVK::new("foo", "v1", "bar"));

let d2: StrDeserializer<SerdeError> = "/v1.bar".into_deserializer();
assert_eq!(GVK::deserialize(d2).unwrap(), GVK::new("", "v1", "bar"));

let d3: StrDeserializer<SerdeError> = "v1.bar".into_deserializer();
assert_eq!(GVK::deserialize(d3).unwrap(), GVK::new("", "v1", "bar"));

let d4: StrDeserializer<SerdeError> = "asdf".into_deserializer();
assert_err!(GVK::deserialize(d4));

let d5: StrDeserializer<SerdeError> = "foo/asdf/asdf".into_deserializer();
assert_err!(GVK::deserialize(d5));
}
}
67 changes: 36 additions & 31 deletions sk-driver/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,38 +62,44 @@ pub fn build_virtual_obj(
original_ns: &str,
virtual_ns: &str,
obj: &DynamicObject,
pod_spec_template_path: &str,
maybe_pod_spec_template_path: Option<&str>,
) -> anyhow::Result<DynamicObject> {
let owner = root;
let mut vobj = obj.clone();
add_common_metadata(&ctx.name, owner, &mut vobj.metadata);
vobj.metadata.namespace = Some(virtual_ns.into());
klabel_insert!(vobj, VIRTUAL_LABEL_KEY => "true");

jsonutils::patch_ext::add(pod_spec_template_path, "metadata", &json!({}), &mut vobj.data, false)?;
jsonutils::patch_ext::add(
&format!("{}/metadata", pod_spec_template_path),
"annotations",
&json!({}),
&mut vobj.data,
false,
)?;
jsonutils::patch_ext::add(
&format!("{}/metadata/annotations", pod_spec_template_path),
ORIG_NAMESPACE_ANNOTATION_KEY,
&json!(original_ns),
&mut vobj.data,
true,
)?;
jsonutils::patch_ext::remove("", "status", &mut vobj.data)?;

// We remove all container ports from the pod specification just before applying, because it is
// _possible_ to create a pod with duplicate container ports, but the apiserver will _reject_ a
// patch containing duplicate container ports. Since pods are mocked out _anyways_ there's no
// reason to expose the ports. We do this here because we still want the ports to be a part of
// the podspec when we're computing its hash, i.e., changes to the container ports will still
// result in changes to the pod in the trace/simulation
jsonutils::patch_ext::remove(&format!("{}/spec/containers/*", pod_spec_template_path), "ports", &mut vobj.data)?;
if let Some(pod_spec_template_path) = maybe_pod_spec_template_path {
jsonutils::patch_ext::add(pod_spec_template_path, "metadata", &json!({}), &mut vobj.data, false)?;
jsonutils::patch_ext::add(
&format!("{}/metadata", pod_spec_template_path),
"annotations",
&json!({}),
&mut vobj.data,
false,
)?;
jsonutils::patch_ext::add(
&format!("{}/metadata/annotations", pod_spec_template_path),
ORIG_NAMESPACE_ANNOTATION_KEY,
&json!(original_ns),
&mut vobj.data,
true,
)?;
jsonutils::patch_ext::remove("", "status", &mut vobj.data)?;

// We remove all container ports from the pod specification just before applying, because it is
// _possible_ to create a pod with duplicate container ports, but the apiserver will _reject_ a
// patch containing duplicate container ports. Since pods are mocked out _anyways_ there's no
// reason to expose the ports. We do this here because we still want the ports to be a part of
// the podspec when we're computing its hash, i.e., changes to the container ports will still
// result in changes to the pod in the trace/simulation
jsonutils::patch_ext::remove(
&format!("{}/spec/containers/*", pod_spec_template_path),
"ports",
&mut vobj.data,
)?;
}

Ok(vobj)
}
Expand Down Expand Up @@ -132,11 +138,7 @@ pub async fn run_trace(ctx: DriverContext, client: kube::Client) -> EmptyResult
ns_api.create(&Default::default(), &vns).await?;
}

let pod_spec_template_path = ctx
.store
.config()
.pod_spec_template_path(&gvk)
.ok_or(anyhow!("unknown simulated object: {:?}", gvk))?;
let pod_spec_template_path = ctx.store.config().pod_spec_template_path(&gvk);
let vobj = build_virtual_obj(&ctx, &root_obj, &original_ns, &virtual_ns, obj, pod_spec_template_path)?;

info!("applying object {}", vobj.namespaced_name());
Expand All @@ -149,8 +151,11 @@ pub async fn run_trace(ctx: DriverContext, client: kube::Client) -> EmptyResult

for obj in &evt.deleted_objs {
info!("deleting object {}", obj.namespaced_name());
let virtual_ns = format!("{}-{}", ctx.virtual_ns_prefix, obj.namespace().unwrap());
let mut vobj = obj.clone();
vobj.metadata.namespace = Some(virtual_ns);
apiset
.api_for_obj(obj)
.api_for_obj(&vobj)
.await?
.delete(&obj.name_any(), &Default::default())
.await?;
Expand Down
12 changes: 11 additions & 1 deletion sk-driver/src/tests/data/trace.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,17 @@
}
}
],
"deleted_objs": []
"deleted_objs": [
{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": {
"name": "nginx-deployment-2",
"namespace": "default"
},
"spec": {}
}
]
}
],
{
Expand Down
6 changes: 6 additions & 0 deletions sk-driver/src/tests/runner_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@ async fn itest_run(#[case] has_start_marker: bool) {
));
then.json_body(status_ok());
})
.handle(|when, then| {
when.method(DELETE).path(format!(
"/apis/apps/v1/namespaces/{TEST_VIRT_NS_PREFIX}-{TEST_NS_NAME}/deployments/nginx-deployment-2"
));
then.json_body(status_ok());
})
.handle(|when, then| {
when.path(format!("/apis/simkube.io/v1/simulationroots/{TEST_DRIVER_ROOT_NAME}"))
.method(DELETE);
Expand Down
4 changes: 2 additions & 2 deletions sk-store/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use sk_core::k8s::GVK;
#[derive(Clone, Debug, Default, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct TrackedObjectConfig {
pub pod_spec_template_path: String,
pub pod_spec_template_path: Option<String>,

#[serde(default, skip_serializing_if = "<&bool>::not")]
pub track_lifecycle: bool,
Expand All @@ -29,7 +29,7 @@ impl TracerConfig {
}

pub fn pod_spec_template_path(&self, gvk: &GVK) -> Option<&str> {
Some(&self.tracked_objects.get(gvk)?.pod_spec_template_path)
self.tracked_objects.get(gvk)?.pod_spec_template_path.as_deref()
}

pub fn track_lifecycle_for(&self, gvk: &GVK) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion sk-store/src/tests/trace_store_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fn tracer() -> TraceStore {
GVK::new("apps", "v1", "Deployment"),
TrackedObjectConfig {
track_lifecycle: true,
pod_spec_template_path: "/spec/template".into(),
pod_spec_template_path: Some("/spec/template".into()),
},
)]),
})
Expand Down
Loading