[Refactor] DCR Monitor Refactoring (1/N) #16
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xxxxavier
reviewed
Oct 31, 2024
xxxxavier
reviewed
Oct 31, 2024
xxxxavier
approved these changes
Nov 1, 2024
privacywill
approved these changes
Nov 4, 2024
dayeol
added a commit
that referenced
this pull request
Nov 6, 2024
2/N PR for a major refactoring of DCR Monitor. See #15. 1/N: #16 # Changes * Remove "UpdateJobStatus" endpoint from the API. Everything must be handled in the reconciler instead. * Added `imagebuilder` interface, which currently has one image builder implementation which is Kaniko * Moved `RunJob` logic into the reconciler. Now TEE is launched by the reconciler via `LaunchInstance` method in `TEEProvider`. * Removed bunch of shallow interfaces from `Config`. Removed bunch of unnecessary global configs, and made them local. * Completely removed KMS logic. We don't need user keys and stage1/stage2 keys in the open source version * Completely removed `PrepareResourcesForUser`. Any logic needed for TEE should go to `TEEProvider`. * Removed `project_number` from config, because it is not needed anymore. * Removed logic for creating/updating workload identity pool provider. It's not used any longer so let's revisit later when we implement data confidentiality. # Caveat * This still doesn't support getting attestation report. # Testing Added unit tests, and also did manual end-to-end testing.
This was referenced Nov 6, 2024
dayeol
added a commit
that referenced
this pull request
Nov 13, 2024
2/3 PR for a major refactoring of DCR Monitor. See #15. 1/3: #16 2/3: #19 This pretty much concludes the major refactoring, resolving #15 # Changes * Added `Created` job status, because the reconciler needs to handle newly created jobs. * Move KanikoService functionality into the monitor (=reconciler) * Remove unused config `Cluster.PodServiceAccount` * Clean up ARG/ENV of TEE dockerfile * In-memory Dockerfile injection in API. See `addDockerfileToTarGz`. This resolves #3. Also Kaniko can just refer to the remote object path. * Removed most of shallow functions in `Config` * Completely removed `CloudProvider` * Removed shallow / no-longer used modules in `pkg`: `pkg/utils/constant.go`, `pkg/utils/file.go`, and `pkg/utils/k8s.go`. # Testing Manual end-to-end testing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a first PR for a major refactoring of DCR monitor. See #15
Warning
This PR breaks attestation functionality, because
TEEProviderdoes not implement attestation feature yet.Changes
CronJobtoDeployment, and have reconciliation loop in the app (cronjob only supports > 1 minute)TEEProvidertype to support different TEE backends. Only partially implemented right now.ListAllInstances,DeleteInstance)reconcileJobfunctionbazel test //...to the CITesting
Manually deployed & tested. Also added a unit test for
updateJobStatusdidn't test
Reconcile, it needs dependency injection for the DB.