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

Bug: Fix Compute Disk Snapshot Schedule #476

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ronballesteros
Copy link

This PR updates the compute disk snapshot schedule module as right now it is requiring all 3 schedule types to be added (see below).

│ Error: Invalid value for input variable
│ 
│   on gcp-snapshots.tf line 26, in module "lnd_lab_disk_snapshots":26:   snapshot_schedule = {
│   27:     daily_schedule = {
│   28:       days_in_cycle = 129:       start_time    = "04:00"30:     }
│   31:   }
│ 
│ The given value is not suitable for module.lndus_disk_snapshots.var.snapshot_schedule declared at .terraform/modules/lndus_disk_snapshots/modules/compute_disk_snapshot/variables.tf:42,1-29:
│ attributes "hourly_schedule" and "weekly_schedule" are required.

The error I'm encountering indicates that only one of daily_schedule, weekly_schedule, or hourly_schedule can be specified in a single google_compute_resource_policy resource's schedule block.

More info: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_resource_policy#nested_snapshot_schedule_policy

snapshot_schedule = {
    daily_schedule = {
      days_in_cycle = 1
      start_time    = "04:00"
    }
    hourly_schedule = {
      hours_in_cycle = 20
      start_time     = "23:00"
    }
    weekly_schedule = {
      day_of_weeks = [
        {
          day        = "MONDAY"
          start_time = "23:00"
        }
      ]
    }
  }
│ "snapshot_schedule_policy.0.schedule.0.weekly_schedule": only one of
│ `snapshot_schedule_policy.0.schedule.0.daily_schedule,snapshot_schedule_policy.0.schedule.0.hourly_schedule,snapshot_schedule_policy.0.schedule.0.weekly_schedule` can be specified, but
│ `snapshot_schedule_policy.0.schedule.0.daily_schedule,snapshot_schedule_policy.0.schedule.0.hourly_schedule,snapshot_schedule_policy.0.schedule.0.weekly_schedule` were specified.

Updated the snapshot variable to ensure that exactly one of daily_schedule, hourly_schedule, or weekly_schedule is provided. It uses a validation rule to enforce that only one of these schedules is specified at a time, preventing any combination of multiple schedules. If none or more than one schedule type is provided, it triggers an error.

Error: Invalid value for variable
│ 
│   on gcp-snapshots.tf line 26, in module "lndus_disk_snapshots":26:   snapshot_schedule = {
│   27:     daily_schedule = {
│   28:       days_in_cycle = 129:       start_time    = "04:00"30:     }
│   31:     hourly_schedule = {
│   32:       hours_in_cycle = 2033:       start_time     = "23:00"34:     }
│   35:     weekly_schedule = {
│   36:       day_of_weeks = [
│   37:         {
│   38:           day        = "MONDAY"39:           start_time = "23:00"40:         }
│   41:       ]
│   42:     }
│   43:   }
│     ├────────────────
│     │ var.snapshot_schedule.daily_schedule is object with 2 attributes
│     │ var.snapshot_schedule.hourly_schedule is object with 2 attributes
│     │ var.snapshot_schedule.weekly_schedule is object with 1 attribute "day_of_weeks"
│ 
│ Exactly one of daily_schedule, hourly_schedule, or weekly_schedule must be provided.
│ 
│ This was checked by the validation rule at .terraform/modules/lndus_disk_snapshots/modules/compute_disk_snapshot/variables.tf:70,3-13.

@ronballesteros ronballesteros requested review from erlanderlo, q2w and a team as code owners January 24, 2025 21:34
Copy link

google-cla bot commented Jan 24, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@ronballesteros
Copy link
Author

Just saw this from @bulowaty: #468. Looks like we're on the same page here.

@ronballesteros
Copy link
Author

Can we get a review on this PR please?

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

Successfully merging this pull request may close these issues.

1 participant