Skip to content
This repository has been archived by the owner on Dec 5, 2020. It is now read-only.

Implementation of Ignition 2.1 #13

Merged
merged 4 commits into from
Sep 6, 2017
Merged

Implementation of Ignition 2.1 #13

merged 4 commits into from
Sep 6, 2017

Conversation

mcuadros
Copy link
Contributor

@mcuadros mcuadros commented Sep 5, 2017

This is the implementation using ignition 2.1.

New resources:
ignition_directory
ignition_link

Updated resources:
ignition_config
ignition_filesystem
ignition_systemd_unit
ignition_user

Fixes #8

@grubernaut
Copy link
Contributor

grubernaut commented Sep 6, 2017

@mcuadros going to trust you've done due diligence here, as it falls on you to maintain this after merging 😂 ❤️ , however would you mind adding a comment with the acceptance test output for the provider? Thanks!

@mcuadros
Copy link
Contributor Author

mcuadros commented Sep 6, 2017

This provider doesn't have any acceptance test, since only generates a JSON string.

@s-urbaniak
Copy link

@mcuadros thanks a lot for the PR! I gave it a shot locally, and have (so far) just one suggestion, namely to mark enable as deprecated:

diff --git a/ignition/resource_ignition_systemd_unit.go b/ignition/resource_ignition_systemd_unit.go
index 3cd2511..ef6de1d 100644
--- a/ignition/resource_ignition_systemd_unit.go
+++ b/ignition/resource_ignition_systemd_unit.go
@@ -15,6 +15,13 @@ func resourceSystemdUnit() *schema.Resource {
                                Required: true,
                                ForceNew: true,
                        },
+                       "enable": {
+                               Type:       schema.TypeBool,
+                               Optional:   true,
+                               Default:    true,
+                               ForceNew:   true,
+                               Deprecated: "Use enabled instead",
+                       },
                        "enabled": {
                                Type:     schema.TypeBool,
                                Optional: true,
@@ -95,6 +102,9 @@ func buildSystemdUnit(d *schema.ResourceData, c *cache) (string, error) {
        }
 
        enabled := d.Get("enabled").(bool)
+       if e, ok := d.GetOk("enable"); ok {
+               enabled = e.(bool)
+       }
 
        return c.addSystemdUnit(&types.Unit{
                Name:     d.Get("name").(string),

@mcuadros
Copy link
Contributor Author

mcuadros commented Sep 6, 2017

@s-urbaniak the changes are massive, on several resources, so I rather not having any deprecated as the create in filesystem, since we are tagging the current master, in order to having two available versions, one for 2.0 and other for 2.1.

@s-urbaniak
Copy link

@mcuadros sounds good! if two separate versions are going to be available, then that is a viable approach.

Thanks again!

@s-urbaniak
Copy link

s-urbaniak commented Sep 6, 2017

@mcuadros another litmus test. ignition failed on during install with the following error:

[    3.319521] ignition[392]: Ignition v0.17.2
[    3.321707] ignition[392]: no config URL provided
[    3.324588] ignition[392]: failed to fetch config: unsupported config version

This fixes it on top of this PR:

diff --git a/ignition/resource_ignition_config.go b/ignition/resource_ignition_config.go
index 7f20943..e3c17bc 100644
--- a/ignition/resource_ignition_config.go
+++ b/ignition/resource_ignition_config.go
@@ -173,6 +173,7 @@ func buildIgnition(d *schema.ResourceData) (types.Ignition, error) {
        var err error
 
        i := types.Ignition{}
+       i.Version = types.MaxVersion.String()
 
        rr := d.Get("replace.0").(map[string]interface{})
        if len(rr) != 0 {

@grubernaut
Copy link
Contributor

Test Output:

$ go test -v ./ignition/...
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestValidateUnit
--- PASS: TestValidateUnit (0.00s)
=== RUN   TestIngnitionFileReplace
--- PASS: TestIngnitionFileReplace (0.02s)
=== RUN   TestIngnitionFileAppend
--- PASS: TestIngnitionFileAppend (0.02s)
=== RUN   TestIngnitionFileReplaceNoVerification
--- PASS: TestIngnitionFileReplaceNoVerification (0.02s)
=== RUN   TestIngnitionFileAppendNoVerification
--- PASS: TestIngnitionFileAppendNoVerification (0.02s)
=== RUN   TestIgnitionConfigDisks
--- PASS: TestIgnitionConfigDisks (0.03s)
=== RUN   TestIgnitionConfigArrays
--- PASS: TestIgnitionConfigArrays (0.03s)
=== RUN   TestIgnitionConfigFilesystems
--- PASS: TestIgnitionConfigFilesystems (0.03s)
=== RUN   TestIgnitionConfigFiles
--- PASS: TestIgnitionConfigFiles (0.03s)
=== RUN   TestIgnitionConfigSystemd
--- PASS: TestIgnitionConfigSystemd (0.03s)
=== RUN   TestIgnitionConfigNetworkd
--- PASS: TestIgnitionConfigNetworkd (0.03s)
=== RUN   TestIgnitionConfigUsers
--- PASS: TestIgnitionConfigUsers (0.03s)
=== RUN   TestIgnitionConfigGroupss
--- PASS: TestIgnitionConfigGroupss (0.03s)
=== RUN   TestIngnitionDirectory
--- PASS: TestIngnitionDirectory (0.03s)
=== RUN   TestIngnitionDisk
--- PASS: TestIngnitionDisk (0.03s)
=== RUN   TestIngnitionDiskResource
--- PASS: TestIngnitionDiskResource (0.03s)
=== RUN   TestIngnitionFile
--- PASS: TestIngnitionFile (0.04s)
=== RUN   TestIngnitionFilesystem
--- PASS: TestIngnitionFilesystem (0.04s)
=== RUN   TestIngnitionGroup
--- PASS: TestIngnitionGroup (0.04s)
=== RUN   TestIngnitionLink
--- PASS: TestIngnitionLink (0.03s)
=== RUN   TestIngnitionNetworkdUnit
--- PASS: TestIngnitionNetworkdUnit (0.03s)
=== RUN   TestIngnitionRaid
--- PASS: TestIngnitionRaid (0.03s)
=== RUN   TestIngnitionSystemdUnit
--- PASS: TestIngnitionSystemdUnit (0.03s)
=== RUN   TestIngnitionSystemdUnitEmptyContentWithDropIn
--- PASS: TestIngnitionSystemdUnitEmptyContentWithDropIn (0.03s)
=== RUN   TestIgnitionSystemdUnit_emptyContent
--- PASS: TestIgnitionSystemdUnit_emptyContent (0.03s)
=== RUN   TestIngnitionUser
--- PASS: TestIngnitionUser (0.04s)
PASS
ok  	github.com/terraform-providers/terraform-provider-ignition/ignition	0.779s

Copy link
Contributor

@grubernaut grubernaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, waiting merge until comments above are resolved, however.

@grubernaut
Copy link
Contributor

@mcuadros this ready to be merged now?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants