From 49c2417c82c733daf1753477f47c36d6892fa4ae Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Tue, 30 Jan 2024 20:19:30 +0530 Subject: [PATCH] refactor: Extract necessary values from spec.dump - If applied, this commit will refactor the code to extract essential values, including container engine, from the spec.dump file. This change replaces the previous approach of retrieving these values from the config.dump file. The spec.dump file stores these values in key-value pairs, providing a more structured and consistent data source. Signed-off-by: Parthiba-Hazra --- cmd/list.go | 14 ++-- internal/config_extractor.go | 65 ++++++++++++------- internal/config_extractor_test.go | 36 +++------- internal/utils.go | 33 ---------- lib/metadata.go | 4 ++ test/checkpointctl.bats | 28 +++++--- .../config.dump | 0 test/data/list_config_spec.dump/spec.dump | 9 +++ 8 files changed, 93 insertions(+), 96 deletions(-) rename test/data/{list_config.dump => list_config_spec.dump}/config.dump (100%) create mode 100644 test/data/list_config_spec.dump/spec.dump diff --git a/cmd/list.go b/cmd/list.go index 00e01d75..b15f5d4d 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -49,7 +49,8 @@ func list(cmd *cobra.Command, args []string) error { "Namespace", "Pod", "Container", - "Time Created", + "Engine", + "Time Checkpointed", "Checkpoint Name", } @@ -71,17 +72,18 @@ func list(cmd *cobra.Command, args []string) error { fmt.Printf("Listing checkpoints in path: %s\n", checkpointPath) for _, file := range files { - namespace, pod, container, timestamp, err := internal.ExtractConfigDump(file) + chkptConfig, err := internal.ExtractConfigDump(file) if err != nil { log.Printf("Error extracting information from %s: %v\n", file, err) continue } row := []string{ - namespace, - pod, - container, - timestamp.Format(time.RFC822), + chkptConfig.Namespace, + chkptConfig.Pod, + chkptConfig.Container, + chkptConfig.ContainerManager, + chkptConfig.Timestamp.Format(time.RFC822), filepath.Base(file), } diff --git a/internal/config_extractor.go b/internal/config_extractor.go index 4308c574..2a5b9ce0 100644 --- a/internal/config_extractor.go +++ b/internal/config_extractor.go @@ -2,54 +2,75 @@ package internal import ( "encoding/json" - "fmt" "log" "os" "path/filepath" - "strings" "time" metadata "github.com/checkpoint-restore/checkpointctl/lib" ) -func ExtractConfigDump(checkpointPath string) (namespace, pod, container string, timestamp time.Time, err error) { +type ChkptConfig struct { + Namespace string + Pod string + Container string + ContainerManager string + Timestamp time.Time +} + +func ExtractConfigDump(checkpointPath string) (*ChkptConfig, error) { tempDir, err := os.MkdirTemp("", "extracted-checkpoint") if err != nil { - return "", "", "", time.Time{}, err + return nil, err } defer os.RemoveAll(tempDir) - filesToExtract := []string{"config.dump"} + filesToExtract := []string{"spec.dump", "config.dump"} if err := UntarFiles(checkpointPath, tempDir, filesToExtract); err != nil { log.Printf("Error extracting files from archive %s: %v\n", checkpointPath, err) - return "", "", "", time.Time{}, err + return nil, err + } + + specDumpPath := filepath.Join(tempDir, "spec.dump") + specContent, err := os.ReadFile(specDumpPath) + if err != nil { + log.Printf("Error reading spec.dump file: %v\n", err) + return nil, err } configDumpPath := filepath.Join(tempDir, "config.dump") - content, err := os.ReadFile(configDumpPath) + configContent, err := os.ReadFile(configDumpPath) if err != nil { log.Printf("Error reading config.dump file: %v\n", err) - return "", "", "", time.Time{}, err + return nil, err } - return extractConfigDumpContent(content) + return extractConfigDumpContent(configContent, specContent) } -func extractConfigDumpContent(content []byte) (namespace, pod, container string, timestamp time.Time, err error) { - var c metadata.ContainerConfig - if err := json.Unmarshal(content, &c); err != nil { - return "", "", "", time.Time{}, err - } +func extractConfigDumpContent(configContent []byte, specContent []byte) (*ChkptConfig, error) { + var spec metadata.Spec + var config metadata.ContainerConfig - parts := strings.Split(c.Name, "_") + if err := json.Unmarshal(configContent, &config); err != nil { + return nil, err + } - if len(parts) >= 4 { - container = parts[1] - pod = parts[2] - namespace = parts[3] - } else { - return "", "", "", time.Time{}, fmt.Errorf("invalid name format") + if err := json.Unmarshal(specContent, &spec); err != nil { + return nil, err } - return namespace, pod, container, c.CheckpointedAt, nil + namespace := spec.Annotations["io.kubernetes.pod.namespace"] + timestamp := config.CheckpointedAt + pod := spec.Annotations["io.kubernetes.pod.name"] + container := spec.Annotations["io.kubernetes.container.name"] + containerManager := spec.Annotations["io.container.manager"] + + return &ChkptConfig{ + Namespace: namespace, + Pod: pod, + Container: container, + ContainerManager: containerManager, + Timestamp: timestamp, + }, nil } diff --git a/internal/config_extractor_test.go b/internal/config_extractor_test.go index 33db6ab2..a9fec06f 100644 --- a/internal/config_extractor_test.go +++ b/internal/config_extractor_test.go @@ -2,50 +2,32 @@ package internal import ( "os" - "path/filepath" "testing" "time" ) -func TestExtractConfigDump(t *testing.T) { - tempDir, err := os.MkdirTemp("", "test-extract-config-dump") +func TestExtractConfigDumpContent(t *testing.T) { + configContent, err := os.ReadFile("../test/data/list_config_spec.dump/config.dump") if err != nil { t.Fatal(err) } - defer os.RemoveAll(tempDir) - - checkpointFile := filepath.Join(tempDir, "checkpoint-123.tar") - if err := CreateTarWithFile(checkpointFile, "../test/data/list_config.dump/config.dump"); err != nil { - t.Fatal(err) - } - namespace, pod, container, timestamp, err := ExtractConfigDump(checkpointFile) + specContent, err := os.ReadFile("../test/data/list_config_spec.dump/spec.dump") if err != nil { - t.Fatalf("ExtractConfigDump failed: %v", err) - } - - expectedNamespace := "default" - expectedPod := "deployment-name" - expectedContainer := "container-name" - expectedTimestamp := time.Date(2024, 1, 28, 0, 10, 45, 673538606, time.FixedZone("", 19800)) - if namespace != expectedNamespace || pod != expectedPod || container != expectedContainer || !timestamp.Equal(expectedTimestamp) { - t.Errorf("ExtractConfigDump returned unexpected values: namespace=%s, pod=%s, container=%s, timestamp=%v", namespace, pod, container, timestamp) + t.Fatal(err) } -} - -func TestExtractConfigDumpContent(t *testing.T) { - content := []byte(`{"id":"6924be1bd90c23f10e2667102b0ee0f74f09bba78b6661871e733cb3b1737821","name":"k8s_container-name_deployment-name_default_6975ee47-6765-45dc-9a2b-1e38d51031f7_0","checkpointedTime":"2024-01-28T00:10:45.673538606+05:30"}`) - namespace, pod, container, timestamp, err := extractConfigDumpContent(content) + chkptConfig, err := extractConfigDumpContent(configContent, specContent) if err != nil { t.Fatalf("ExtractConfigDumpContent failed: %v", err) } expectedNamespace := "default" - expectedPod := "deployment-name" + expectedPod := "pod-name" expectedContainer := "container-name" + expectedContainerManager := "cri-o" expectedTimestamp := time.Date(2024, 1, 28, 0, 10, 45, 673538606, time.FixedZone("", 19800)) - if namespace != expectedNamespace || pod != expectedPod || container != expectedContainer || !timestamp.Equal(expectedTimestamp) { - t.Errorf("ExtractConfigDumpContent returned unexpected values: namespace=%s, pod=%s, container=%s, timestamp=%v", namespace, pod, container, timestamp) + if chkptConfig.Namespace != expectedNamespace || chkptConfig.Pod != expectedPod || chkptConfig.Container != expectedContainer || !chkptConfig.Timestamp.Equal(expectedTimestamp) || chkptConfig.ContainerManager != expectedContainerManager { + t.Errorf("ExtractConfigDumpContent returned unexpected values: namespace=%s, pod=%s, container=%s, timestamp=%v", chkptConfig.Namespace, chkptConfig.Pod, chkptConfig.Container, chkptConfig.Timestamp) } } diff --git a/internal/utils.go b/internal/utils.go index 446a819c..fc70d9be 100644 --- a/internal/utils.go +++ b/internal/utils.go @@ -1,10 +1,8 @@ package internal import ( - "archive/tar" "fmt" "os" - "path/filepath" "strings" "time" @@ -84,34 +82,3 @@ func CleanupTasks(tasks []Task) { } } } - -func CreateTarWithFile(tarPath string, filePath string) error { - file, err := os.Create(tarPath) - if err != nil { - return err - } - defer file.Close() - tarWriter := tar.NewWriter(file) - defer tarWriter.Close() - - fileInfo, err := os.Stat(filePath) - if err != nil { - return err - } - header, err := tar.FileInfoHeader(fileInfo, "") - if err != nil { - return err - } - header.Name = filepath.Base(filePath) - if err := tarWriter.WriteHeader(header); err != nil { - return err - } - content, err := os.ReadFile(filePath) - if err != nil { - return err - } - if _, err := tarWriter.Write(content); err != nil { - return err - } - return nil -} diff --git a/lib/metadata.go b/lib/metadata.go index 70497a0b..f64d12d1 100644 --- a/lib/metadata.go +++ b/lib/metadata.go @@ -48,6 +48,10 @@ type ContainerConfig struct { Restored bool `json:"restored"` } +type Spec struct { + Annotations map[string]string `json:"annotations,omitempty"` +} + type ContainerdStatus struct { CreatedAt int64 StartedAt int64 diff --git a/test/checkpointctl.bats b/test/checkpointctl.bats index 5629b4d9..d2fdc2f8 100644 --- a/test/checkpointctl.bats +++ b/test/checkpointctl.bats @@ -656,27 +656,39 @@ function teardown() { @test "Run checkpointctl list with empty tar file" { touch "$TEST_TMP_DIR1"/checkpoint-nginx-empty.tar - checkpointctl list -p "$TEST_TMP_DIR1"/ + checkpointctl list -p "$TEST_TMP_DIR1" [ "$status" -eq 0 ] - [[ "${lines[1]}" == *"Error reading config.dump file"* ]] + [[ "${lines[1]}" == *"Error reading spec.dump file"* ]] [[ "${lines[2]}" == *"Error extracting information"* ]] } -@test "Run checkpointctl list with tar file with empty config.dump" { +@test "Run checkpointctl list with tar file with valid spec.dump and empty config.dump" { touch "$TEST_TMP_DIR1"/config.dump + cp data/list_config_spec.dump/spec.dump "$TEST_TMP_DIR1" mkdir "$TEST_TMP_DIR1"/checkpoint - ( cd "$TEST_TMP_DIR1" && tar cf "$TEST_TMP_DIR2"/checkpoint-empty-config.tar . ) + ( cd "$TEST_TMP_DIR1" && tar cf "$TEST_TMP_DIR2"/checkpoint-config.tar . ) checkpointctl list -p "$TEST_TMP_DIR2" [ "$status" -eq 0 ] - [[ ${lines[1]} == *"Error extracting information from $TEST_TMP_DIR2/checkpoint-empty-config.tar: unexpected end of JSON input" ]] + [[ "${lines[1]}" == *"Error extracting information from $TEST_TMP_DIR2/checkpoint-config.tar: unexpected end of JSON input"* ]] } -@test "Run checkpointctl list with tar file with valid config.dump" { - cp data/list_config.dump/config.dump "$TEST_TMP_DIR1" +@test "Run checkpointctl list with tar file with valid config.dump and empty spec.dump" { + touch "$TEST_TMP_DIR1"/spec.dump + cp data/list_config_spec.dump/config.dump "$TEST_TMP_DIR1" + mkdir "$TEST_TMP_DIR1"/checkpoint + ( cd "$TEST_TMP_DIR1" && tar cf "$TEST_TMP_DIR2"/checkpoint-config.tar . ) + checkpointctl list -p "$TEST_TMP_DIR2" + [ "$status" -eq 0 ] + [[ ${lines[1]} == *"Error extracting information from $TEST_TMP_DIR2/checkpoint-config.tar: unexpected end of JSON input" ]] +} + +@test "Run checkpointctl list with tar file with valid config.dump nad spec.dump" { + cp data/list_config_spec.dump/config.dump "$TEST_TMP_DIR1" + cp data/list_config_spec.dump/spec.dump "$TEST_TMP_DIR1" mkdir "$TEST_TMP_DIR1"/checkpoint ( cd "$TEST_TMP_DIR1" && tar cf "$TEST_TMP_DIR2"/checkpoint-valid-config.tar . ) checkpointctl list -p "$TEST_TMP_DIR2" [ "$status" -eq 0 ] - [[ "${lines[4]}" == *"| default | deployment-name | container-name |"* ]] + [[ "${lines[4]}" == *"| default | pod-name | container-name | cri-o |"* ]] [[ "${lines[4]}" == *"| checkpoint-valid-config.tar |"* ]] } \ No newline at end of file diff --git a/test/data/list_config.dump/config.dump b/test/data/list_config_spec.dump/config.dump similarity index 100% rename from test/data/list_config.dump/config.dump rename to test/data/list_config_spec.dump/config.dump diff --git a/test/data/list_config_spec.dump/spec.dump b/test/data/list_config_spec.dump/spec.dump new file mode 100644 index 00000000..6ea5341c --- /dev/null +++ b/test/data/list_config_spec.dump/spec.dump @@ -0,0 +1,9 @@ +{ + "annotations": { + "io.container.manager": "cri-o", + "io.kubernetes.container.hash": "1511917a", + "io.kubernetes.container.name": "container-name", + "io.kubernetes.pod.name": "pod-name", + "io.kubernetes.pod.namespace": "default" + } +} \ No newline at end of file