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

Add AccessPoint resource for EFS #1201

Merged

Conversation

EdgeJ
Copy link
Contributor

@EdgeJ EdgeJ commented Mar 11, 2022

Signed-off-by: EdgeJ 5093048+EdgeJ@users.noreply.github.com

Description of your changes

Adds the AccessPoint resource for the EFS family by removing the ignore parameters in the code generation config and auto-generating the main controller code.

Fixes #1182

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Example yaml configuration added to the repo. Creation and deletion have been tested by manually applying to a local k8s cluster and verifying created infrastructure.

@zonybob
Copy link
Contributor

zonybob commented Mar 14, 2022

@EdgeJ thanks again for this PR! Looking forward to having AccessPoints soon.

I checked out your branch, built and pushed to my cluster. The initial attempt to create an AccessPoint was successful in the sense that the AWS resource was indeed created and becomes available, however the AccessPoint managed resource itself never becomes Ready: true ... it is stuck at Ready: false for Reason: Creating

UPDATE:
(I admittedly have not done a new resource type before, so I am learning)
It looks like we're just missing a postObserve
https://github.com/Simspace/provider-aws/blob/edgej/efs-access-point-resource/CODE_GENERATION.md#readiness-check

After adding that, it seems better (although I am not certain if what I did was the "most correct" for Access Points.)

func postObserve(_ context.Context, cr *svcapitypes.AccessPoint, resp *svcsdk.DescribeAccessPointsOutput, obs managed.ExternalObservation, err error) (managed.ExternalObservation, error) {
	if err != nil {
		return managed.ExternalObservation{}, err
	}
	switch aws.StringValue(resp.AccessPoints[0].LifeCycleState) {
	case string(svcapitypes.LifeCycleState_available):
		cr.SetConditions(xpv1.Available())
	case string(svcapitypes.LifeCycleState_creating):
		cr.SetConditions(xpv1.Creating())
	case string(svcapitypes.LifeCycleState_deleting):
		cr.SetConditions(xpv1.Deleting())
	case string(svcapitypes.LifeCycleState_error):
		cr.SetConditions(xpv1.Unavailable())
	case string(svcapitypes.LifeCycleState_deleted):
		cr.SetConditions(xpv1.Unavailable())
	}

	return obs, nil
}

@EdgeJ
Copy link
Contributor Author

EdgeJ commented Mar 14, 2022

I checked out your branch, built and pushed to my cluster. The initial attempt to create an AccessPoint was successful in the sense that the AWS resource was indeed created and becomes available, however the AccessPoint managed resource itself never becomes Ready: true ... it is stuck at Ready: false for Reason: Creating

UPDATE: (I admittedly have not done a new resource type before, so I am learning) It looks like we're just missing a postObserve https://github.com/Simspace/provider-aws/blob/edgej/efs-access-point-resource/CODE_GENERATION.md#readiness-check

After adding that, it seems better (although I am not certain if what I did was the "most correct" for Access Points.)

Thanks @zonybob! I hadn't caught that it wasn't becoming ready. I was working from memory on how to set up the auto-generated resources and forgot about setting up readiness. I'll add the readiness check today and push the changes up.

Signed-off-by: EdgeJ <5093048+EdgeJ@users.noreply.github.com>
@EdgeJ EdgeJ force-pushed the edgej/efs-access-point-resource branch from a190afd to 1f3de35 Compare March 14, 2022 18:36
Signed-off-by: EdgeJ <5093048+EdgeJ@users.noreply.github.com>
@EdgeJ EdgeJ force-pushed the edgej/efs-access-point-resource branch from 7f9f569 to daf4d9f Compare March 14, 2022 19:15
Signed-off-by: EdgeJ <5093048+EdgeJ@users.noreply.github.com>
@zonybob
Copy link
Contributor

zonybob commented Apr 6, 2022

Just checking on status of this pull request. Need any more testing or anything? Do we have a target release in mind? Thanks!

Copy link
Member

@haarchri haarchri left a comment

Choose a reason for hiding this comment

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

tested the AccessPoint working as expected - thanks for implementation - LGTM

@haarchri haarchri merged commit bb710b3 into crossplane-contrib:master Apr 15, 2022
@EdgeJ EdgeJ deleted the edgej/efs-access-point-resource branch May 11, 2022 18:51
febarbosa182 pushed a commit to febarbosa182/provider-aws that referenced this pull request May 23, 2022
* Add AccessPoint resource for EFS

Signed-off-by: EdgeJ <5093048+EdgeJ@users.noreply.github.com>
Signed-off-by: Felipe Barbosa <lybrbarbosa@gmail.com>
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.

Support EFS Access Points
3 participants