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

generate_name prefix seems to not be honored #2539

Closed
jforest opened this issue Aug 18, 2023 · 5 comments · Fixed by #2808
Closed

generate_name prefix seems to not be honored #2539

jforest opened this issue Aug 18, 2023 · 5 comments · Fixed by #2808
Assignees
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed

Comments

@jforest
Copy link

jforest commented Aug 18, 2023

What happened?

I have both a pv and pvc being created, I specify the pulumi_kubernetes.meta.v1.ObjectMetaArgs generate_name attribute on each which should specify a prefix for the name. But the name that results is not using my specified prefix, it appears to be using the resource_name of the object as the prefix instead.

Expected Behavior

For example, my code for generating a pvc from the pv I created before

        # Create the PVC from the PV
        self.pvc = pulumi_kubernetes.core.v1.PersistentVolumeClaim(
            f'{self.cluster.name}-workbench-pvc',
            args=pulumi_kubernetes.core.v1.PersistentVolumeClaimInitArgs(
                metadata=pulumi_kubernetes.meta.v1.ObjectMetaArgs(
                    generate_name='workbench-pvc',
                    namespace='datascience'
                ),
                spec=pulumi_kubernetes.core.v1.PersistentVolumeClaimSpecArgs(
                    access_modes=['ReadWriteMany'],
                    resources=pulumi_kubernetes.core.v1.ResourceRequirementsArgs(
                        requests=capacity
                    ),
                    storage_class_name='',
                    volume_name=pv.metadata.name
                )
            ),
            opts=pulumi.ResourceOptions(provider=self.cluster.provider,
                                        parent=pv)
        )

I would expect the name of the pvc to be workbench-pvc-<randomstring> but instead I get <cluster_name>-workbench-pvc-<randomstring>

our cluster name is internal-eks01 in this case

internal-eks01-workbench-pvc-c1aa4e6e         Bound    internal-eks01-workbench-pv-abae85d9         1200Gi     RWX                           55d

Steps to reproduce

Create a pvc from a pv using the code below

       # Create the PVC from the PV
       self.pvc = pulumi_kubernetes.core.v1.PersistentVolumeClaim(
           f'{self.cluster.name}-workbench-pvc',
           args=pulumi_kubernetes.core.v1.PersistentVolumeClaimInitArgs(
               metadata=pulumi_kubernetes.meta.v1.ObjectMetaArgs(
                   generate_name='workbench-pvc',
                   namespace='datascience'
               ),
               spec=pulumi_kubernetes.core.v1.PersistentVolumeClaimSpecArgs(
                   access_modes=['ReadWriteMany'],
                   resources=pulumi_kubernetes.core.v1.ResourceRequirementsArgs(
                       requests=capacity
                   ),
                   storage_class_name='',
                   volume_name=pv.metadata.name
               )
           ),
           opts=pulumi.ResourceOptions(provider=self.cluster.provider,
                                       parent=pv)
       )

it will not use the generate_name prefix, it instead uses the resource_name as a prefix

Output of pulumi about

❯ pulumi about
CLI
Version      3.77.1
Go Version   go1.20.7
Go Compiler  gc

Plugins
NAME                     VERSION
aws                      5.41.0
aws-native               0.72.0
kubernetes               3.28.1
kubernetes-cert-manager  0.0.5
postgresql               3.8.0
python                   unknown

Host
OS       darwin
Version  12.6.8
Arch     arm64

This project is written in python: executable='/Users/jforest/.pyenv/shims/python3' version='3.9.11
'

<SNIP SOME RESOURCES OUT>

Found no pending operations associated with jforest

Current Stack: jforest/aws-main-services/jforest
Backend
Name           pulumi.com
URL            https://app.pulumi.com/jforest
User           jforest
Organizations  jforest

Additional context

versions of pulumi libraries:

❯ pip list | grep pulumi
pulumi                         3.68.0
pulumi-aws                     5.41.0
pulumi-aws-native              0.72.0
pulumi-kubernetes              3.28.1
pulumi-kubernetes-cert-manager 0.0.5
pulumi-postgresql              3.8.0

I would like to upgrade pulumi-kubernetes to 4.1.0, but pulumi-kubernetes-cert-manager will not allow versions greater than 3.x. pulumi/pulumi-kubernetes-cert-manager#21 and pulumi/pulumi-kubernetes-cert-manager#22

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@jforest jforest added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Aug 18, 2023
@rquitales
Copy link
Contributor

Hello @jforest,

Thank you for raising this issue. In Kubernetes, the generateName field is utilized to generate a name for a resource when it's sent for creation to the K8s API Server and doesn't already have a name field set. This name generation happens on the K8s server's side.

However, in Pulumi, when resources don't explicitly contain a name field, they are auto-named by default. This Pulumi behavior is causing the generateName field to be ignored when sending resources to the K8s API server, as Pulumi's auto-naming process generates and sets a name which is then sent to the API server, thus overriding whatever is set in generateName.

To resolve this issue, we will need to implement a check to ensure that auto-naming doesn't occur when the generateName field is supplied. This will allow the generateName field to function as intended.

@rquitales rquitales removed the needs-triage Needs attention from the triage team label Aug 18, 2023
@gunzy83
Copy link

gunzy83 commented Aug 29, 2023

I have also run into this. I am going to work around it by generating our own names but I would appreciate if a fix could be prioritised. This should operate in the same way as namePrefix in the AWS provider.

@mikhailshilkov mikhailshilkov added this to the 0.94 milestone Sep 1, 2023
@zlepper
Copy link

zlepper commented Sep 6, 2023

I would add another reason for prioritizing this: If you have several levels of nested component resources, then the names pulumi generates can become so long they are rejected by the apiserver.

@EronWright EronWright self-assigned this Sep 14, 2023
@mikhailshilkov mikhailshilkov modified the milestones: 0.94, 0.95 Sep 29, 2023
@EronWright

This comment was marked as outdated.

@mikhailshilkov mikhailshilkov added kind/enhancement Improvements or new features and removed kind/bug Some behavior is incorrect or out of spec labels Oct 3, 2023
@EronWright EronWright removed this from the 0.95 milestone Oct 3, 2023
EronWright added a commit that referenced this issue Feb 2, 2024
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
    for Pulumi's contribution guidelines.

    Help us merge your changes more quickly by adding more details such
    as labels, milestones, and reviewers.-->

### Proposed changes

<!--Give us a brief description of what you've done and what it solves.
-->

This PR changes the provider's await logic to get the object name from
the output properties rather than the input properties, since the inputs
may or may not contain an object name, to prepare to support
`generateName` field (see
#2539). Note that
auto-naming populates the inputs, whereas generateName doesn't.

The awaiters continue to look to the inputs (not the live object) for
the `skipAwait` and `timeout` annotations.

### Related issues (optional)

<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->

Related: #2539
EronWright added a commit that referenced this issue Feb 3, 2024
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
    for Pulumi's contribution guidelines.

    Help us merge your changes more quickly by adding more details such
    as labels, milestones, and reviewers.-->

### Proposed changes

<!--Give us a brief description of what you've done and what it solves.
-->

This PR implements support for `.metadata.generateName` in CSA mode,
based on #2790.
1. Adjust the auto-naming logic to consider `.metadata.generateName` to
be a variant of auto-naming. Pulumi will not assign an auto-name to a
new resource that has `generateName`, and upon delete will use the
replace-then-delete technique.
2. Use outputs rather than inputs to fetch the live object or to obtain
the live object's name.
3. Fix an autonaming bug where the old auto-name would erroneously be
adopted, overwriting a new computed name.

### Tests
1. A new integration test is provided to test the use of
`metadata.generateName`. It tests creation, update, replacement, and
promotion from `.generateName` to `.name`.
([ref](https://github.com/pulumi/pulumi-kubernetes/pull/2808/files#diff-7e85ce707283b1d281d971a8e5b0c49b959b0803ba3437dc9dbd422552326835R269))
2. New test cases for the await logic.
3. The existing autonaming test is enhanced to test how autonaming takes
precedence over `generateName`, in the update case. This is to ensure
backwards compatibility, e.g. in the edge case that an existing object
has a `generateName` field (which Pulumi ignored until now).

### Related issues (optional)
Closes #2539
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Feb 3, 2024
@gunzy83
Copy link

gunzy83 commented Apr 19, 2024

I see this has been closed but I can't seem to find any record of why this was only fixed for CSA and not SSA (and disallows the field in YAML rendering mode). Is there a reason or will be this be supported later?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Projects
None yet
7 participants