-
Notifications
You must be signed in to change notification settings - Fork 45
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
fix: deploy to a closed channel #661
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -410,6 +410,13 @@ func (c applicationsClient) deployFromRepository(applicationAPIClient Applicatio | |
return errors.Join(errs...) | ||
} | ||
|
||
// publishing to a closed channel automatically upgrade the channel. And this could | ||
// lead to an inconsistent terraform state. This check is to error out to the user, | ||
// so they can purposely select the track. | ||
if deployInfo.Channel != transformedInput.charmChannel { | ||
return fmt.Errorf("you are trying to publish to a closed channel. Check the channel for the %s", transformedInput.applicationName) | ||
Comment on lines
+416
to
+417
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're failing after the application has been deployed, but no local OCI image resources are uploaded? Thus the terraform state will not be saved, but the application exists in juju. This sounds like a worse state to be in than the one you're trying to solve. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good |
||
} | ||
|
||
fileSystem := osFilesystem{} | ||
// Upload the provided local resources to Juju | ||
uploadErr := uploadExistingPendingResources(deployInfo.Name, localPendingResources, fileSystem, resourceAPIClient) | ||
|
@@ -1407,13 +1414,16 @@ func (c applicationsClient) computeSetCharmConfig( | |
msg := fmt.Sprintf("the new charm does not support the current architecture %q", oldOrigin.Architecture) | ||
return nil, errors.New(msg) | ||
} | ||
|
||
// Ensure the new revision or channel is contained | ||
// in the origin to be saved by juju when AddCharm | ||
// is called. | ||
if input.Revision != nil { | ||
oldOrigin.Revision = input.Revision | ||
} else if input.Channel != "" { | ||
if newOrigin.Risk != resolvedOrigin.Risk { | ||
msg := "you've requested a risk different from the one resolved. Probably the channel was close" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'm not too fond of this message - it needs rewording There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yy, just want to show the approach before polishing the pr. |
||
return nil, errors.New(msg) | ||
} | ||
oldOrigin.Track = newOrigin.Track | ||
oldOrigin.Risk = newOrigin.Risk | ||
oldOrigin.Branch = newOrigin.Branch | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -307,6 +307,34 @@ func TestAcc_CharmUpdateBase(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestClosedChannel(t *testing.T) { | ||
modelName := acctest.RandomWithPrefix("tf-test-charmbaseupdates") | ||
|
||
resource.ParallelTest(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
ProtoV6ProviderFactories: frameworkProviderFactories, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: fmt.Sprintf(` | ||
resource "juju_model" "this" { | ||
name = %q | ||
} | ||
|
||
resource "juju_application" "k8s" { | ||
name = "k8s" | ||
model = juju_model.this.name | ||
charm { | ||
name = "k8s" | ||
channel = "1.31/beta" | ||
base = "[email protected]" | ||
} | ||
} | ||
`, modelName), | ||
ExpectError: regexp.MustCompile(".*closed channel.*")}, | ||
}, | ||
}) | ||
} | ||
|
||
func TestAcc_ResourceRevisionUpdatesLXD(t *testing.T) { | ||
if testingCloud != LXDCloudTesting { | ||
t.Skip(t.Name() + " only runs with LXD") | ||
|
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.
in this case the terraform plan is specifying a closed channel, so trying to deploy from a channel, not publish