saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.19k stars 5.48k forks source link

[BUG] salt-cloud for ec2 generates minion keys before doing validation (invalid ami) #63845

Open dfidler opened 1 year ago

dfidler commented 1 year ago

Description

If you create a profile that includes an AMI, but that AMI has rotated out (which is common), minion keys are created, but the build fails.

Setup

I'm going to assume that you have a working ec2 account and salt-cloud provider...

# /etc/salt/cloud.profiles.d
ec2-centos7-us-west-2:
  provider: ec2
  size: t2.micro
  ssh_username: centos-test
  location: us-west-2
  image: somethinginvalid
  script_args: stable latest
  network_interfaces:
    - DeviceIndex: 0
      PrivateIpAddresses:
        - Primary: True
      #auto assign public ip, not EIP
      AssociatePublicIpAddress: True
  #  script_args: stable 3004.2
  #  grains:
  #    role: demo_app1

Please be as specific as possible and give set-up details.

Steps to Reproduce the behavior

  1. Configure the profile per "Setup"

  2. Update to the latest salt-cloud version

    # salt-cloud -u
    Success:
    ----------
    Files updated:
        - /etc/salt/cloud.deploy.d/bootstrap-salt.sh
        - /usr/lib/python3.6/site-packages/salt/cloud/deploy/bootstrap-salt.sh
  3. Build the instance with salt-cloud

    # salt-cloud -p ec2-centos7-us-west-2 invalidtest -l info
    [INFO    ] salt-cloud starting
    [INFO    ] Creating Cloud VM invalidtest in us-west-2
    [ERROR   ] There was a profile error: list index out of range
  4. Check your minion keys

    # salt-key
    Accepted Keys:
    invalidtest
    saltmaster
    Denied Keys:
    Unaccepted Keys:
    Rejected Keys:
  5. Also, I'll raise this as another bug to track separately, but if you try to destroy that salt-key using salt-cloud, it doesn't clean up the key

    
    # salt-cloud -yd invalidtest -l info
    [INFO    ] salt-cloud starting
    No machines were found to be destroyed

salt-key

Accepted Keys: invalidtest saltmaster Denied Keys: Unaccepted Keys: Rejected Keys:


**Expected behavior**

The order of build operations should be such that the minimum number of pre-validation checks need to be written, while also ensuring that objects don't need to be cleaned up after a failure.

For instance, there are two fundamental sections in building a VM.
1. Create the VM - this has many dependencies (AWS creds work, VPC exists, subnets exist, security groups exist, etc, etc, etc).  Because of the complexity of the cloud provider's APIs I don't think we should pre-check for all of them as the process of making the vm creation request will spit out the appropriate errors
2. Installing the minion - (pre-generate keys, remote login to system, install minion, configure master, start); in order to successfully perform this function:
  - does the ssh key exist  

So, in my head, the order of operations should be:
1. Pre-validation
    - keypair exists
    - private key or password supplied 
2. Create VM
4. Pre-create minion keys
5. Install minion

**Versions Report**
<details><summary>salt --versions-report</summary>

salt --versions-report

Salt Version: Salt: 3004.2

Dependency Versions: cffi: 1.9.1 cherrypy: unknown dateutil: Not Installed docker-py: Not Installed gitdb: 0.6.4 gitpython: 1.0.2 Jinja2: 2.11.1 libgit2: Not Installed M2Crypto: 0.35.2 Mako: Not Installed msgpack: 0.6.2 msgpack-pure: Not Installed mysql-python: Not Installed pycparser: 2.14 pycrypto: Not Installed pycryptodome: 3.16.0 pygit2: Not Installed Python: 3.6.8 (default, Nov 16 2020, 16:55:22) python-gnupg: Not Installed PyYAML: 3.13 PyZMQ: 17.0.0 smmap: 0.9.0 timelib: Not Installed Tornado: 4.5.3 ZMQ: 4.1.4

Salt Extensions: SSEAPE: 8.10.2.4

System Versions: dist: centos 7 Core locale: UTF-8 machine: x86_64 release: 3.10.0-957.1.3.el7.x86_64 system: Linux version: CentOS Linux 7 Core


**salt-cloud versions report:**

salt-cloud --version

salt-cloud 3004.2

/bin/bash /etc/salt/cloud.deploy.d/bootstrap-salt.sh -v

/etc/salt/cloud.deploy.d/bootstrap-salt.sh -- Version 2022.10.04

/bin/bash /usr/lib/python3.6/site-packages/salt/cloud/deploy/bootstrap-salt.sh -v

/usr/lib/python3.6/site-packages/salt/cloud/deploy/bootstrap-salt.sh -- Version 2022.10.04


</details>

**Additional context**
Add any other context about the problem here.
Ch3LL commented 1 year ago

@dfidler It should delete the key if there is a failure. Is that not happening?

dfidler commented 1 year ago

@Ch3LL correct, the key is not being deleted