-
Notifications
You must be signed in to change notification settings - Fork 53
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
cli: enable constellation apply
to create new clusters
#2549
Conversation
✅ Deploy Preview for constellation-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
90cea7f
to
5f5e84e
Compare
constellation apply
constellation apply
to create new clusters
0e83ed8
to
54aecef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm. I am not very familiar with the TF code right now, so I am requesting @elchead as well.
Maybe it makes sense at some point to reuse the pattern Adrian introduced in the attestationconfig cli. It allows unittesting the correct order of operations. And might simplify the apply function. I have sth like this in mind: type Phase interface {
func Apply() error
}
func NewPhases(skips map[skipPhase]{}) ([]Phase, error){
// determine which phases to execute here.
// call constructor for each phase.
// don't use reflection, hardcode each constructor call.
}
func main() {
phases, err := NewPhases(map[skipPhase]{})
if err != nil {
log.Fatal(err)
}
for _, phase := range phases {
if err := phase.Apply(); err != nil {
log.Fatal(err)
}
}
} But I am not sure yet if this is needed. Most phases are just a single command. Imo the main goal would be to separate phase execution from determining which phase to run. |
cli/internal/cmd/apply.go
Outdated
@@ -517,10 +530,18 @@ func (a *applyCmd) validateInputs(cmd *cobra.Command, configFetcher attestationc | |||
cmd.PrintErrln("WARNING: Attestation temporarily relies on AWS nitroTPM. See https://docs.edgeless.systems/constellation/workflows/config#choosing-a-vm-type for more information.") | |||
} | |||
|
|||
if !a.flags.skipPhases.contains(skipInitPhase) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is it just me, but I find this easier to read;
skip suggests we should skip it. (feel free to ignore)
if !a.flags.skipPhases.contains(skipInitPhase) { | |
if !a.flags.skipPhases.skip(skipInitPhase) { |
8692273
to
f770d1b
Compare
Updated the docs to remove |
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Co-authored-by: Thomas Tendyck <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
Signed-off-by: Daniel Weiße <[email protected]>
f770d1b
to
15de42b
Compare
Coverage report
|
Context
The
constellation apply
command should be the one command to manage a Constellation cluster.This PR brings the functionality of another command (
constellation create
) toapply
.Proposed change(s)
constellation apply
for creating new cluster, replacingconstellation create
Additional info
constellation apply
in the futureChecklist