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

load cluster manifest and lock file #2334

Closed
3 tasks
xenowits opened this issue Jun 19, 2023 · 5 comments
Closed
3 tasks

load cluster manifest and lock file #2334

xenowits opened this issue Jun 19, 2023 · 5 comments
Assignees
Labels
protocol Protocol Team tickets

Comments

@xenowits
Copy link
Contributor

🎯 Problem to be solved

Currently, we have two files representing charon's config:

  • cluster-lock.json: Legacy lock file (old)
  • cluster-manifest.pb: Mutable cluster manifest (new)

We need a way to figure out what happens if both of these files are provided.

Proposed solution

Refactor the manifest.Load function to take 2 parameters, manifestFile & legacyLockFile:

func Load(manifestFile, legacyLockFile string, lockCallback func(cluster.Lock) error) (*manifestpb.Cluster, error) {}

The following is the intended flow:

  • Try loading the manifest file first.
    • If successful, use the manifest and return
  • If manifest file is not found, try loading the legacyLockFile
    • If successful, use the lock and return
  • If none of the files are found, return an error

🧪 Tests

  • Tested by new automated unit/integration/smoke tests
  • Manually tested on core team/canary/test clusters
  • Manually tested on local compose simnet

👐 Additional acceptance criteria

None

❌ Out of Scope

None

@xenowits xenowits self-assigned this Jun 19, 2023
@github-actions github-actions bot added the protocol Protocol Team tickets label Jun 19, 2023
@OisinKyne
Copy link
Contributor

I think this is a false problem. What is the problem we are really trying to solve (upstream of this issue?). I don't think we should be designing for a case where we have both files present. Or at least I don't see how this solves a customer problem and doesn't permit tech debt we should instead remove.

@gsora
Copy link
Collaborator

gsora commented Jun 23, 2023

@OisinKyne how would you handle the situation in which a user wants to load a cluster created in v0.16 in v0.17, assuming we only support either lock or manifest files?

This is what we're trying to solve: supporting old cluster lock files on charon versions that support cluster manifest.

An alternative might be writing a migration tool/command, but that opens a can of worms I don't think we should close.

I believe we should support cluster lock files at some point, but we can live with this bit of tech debt right now.

@corverroos
Copy link
Contributor

Another safety feature could be:

If both legacy lock and manifest files are found, ensure that the cluster hashes match, ie, that the manifest contains that legacy lock. Else error with mismatching manifest and legacy lock found

@xenowits
Copy link
Contributor Author

The functionality to load from both files would help us in slowly phasing out clusters with legacy lock files. Since we won't be building a migration tool to bump "legacy clusters" to "manifest clusters", this feature would help in easy migration:

  • load legacy lock
  • run any alpha command, like charon alpha add-validators-solo
  • a new cluster-manifest.pb file is created
  • this new cluster manifest can now be used to run charon

@xenowits
Copy link
Contributor Author

Another safety feature could be:

If both legacy lock and manifest files are found, ensure that the cluster hashes match, ie, that the manifest contains that legacy lock. Else error with mismatching manifest and legacy lock found

#2335

obol-bulldozer bot pushed a commit that referenced this issue Jun 26, 2023
Loads cluster either from `cluster manifest` or `legacy lock` file. If both files are provided, `cluster manifest` is read first.

category: feature 
ticket: #2334
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol Protocol Team tickets
Projects
None yet
Development

No branches or pull requests

4 participants