Skip to content
This repository has been archived by the owner on Jul 25, 2022. It is now read-only.

fix gardenctl ssh alicloud node timed out issue #380

Merged

Conversation

neo-liang-sap
Copy link
Contributor

@neo-liang-sap neo-liang-sap commented Oct 13, 2020

What this PR does / why we need it:
fix gardenctl ssh alicloud node timed out issue
Which issue(s) this PR fixes:
Fixes #379

Special notes for your reviewer:

Some explanation on this PR:

  1. please upgrade your aliyun cli to latest
  2. in currently ssh logic of alicloud, when the SecurityGroup exist, no Rules will created. There are two cases which prevents ssh alicloud node success:
    a. the SG exist but no rules exist
    b. the SG exist, rules exist but the SourceIPCidr in rules is incorrect
  3. so this PR enforce creation of rules no matter SG exist
  4. from my test, duplicating creating rules with same SourceIPCidr,port,protocol,ingress/egress won't causing multi records in SG, so the creation of rules is idempotent
  5. i refactored
_, err = ExecCmdReturnOutput("bash", "-c", "aliyun ecs AuthorizeSecurityGroup --Policy Accept --NicType intranet --Priority 1 --SourceCidrIp %s --PortRange 22/22 --IpProtocol tcp --SecurityGroupId="+a.BastionSecurityGroupID, a.MyPublicIP)

to

createSGCmdString := "aliyun ecs AuthorizeSecurityGroup --Policy Accept --NicType intranet --Priority 1 --SourceCidrIp " + a.MyPublicIP + " --PortRange 22/22 --IpProtocol tcp --SecurityGroupId=" + a.BastionSecurityGroupID
		_, err = ExecCmdReturnOutput("bash", "-c", createSGCmdString)

as previous code causing following error
image

Release note:

fix gardenctl ssh alicloud node timed out issue

@neo-liang-sap neo-liang-sap requested a review from a team as a code owner October 13, 2020 03:49
@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Oct 13, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 13, 2020
@neo-liang-sap
Copy link
Contributor Author

neo-liang-sap commented Oct 13, 2020

tested in several alicloud seeds and shoots (including SG exist and non-exist)
image
image
image
image
image

@gardener-robot gardener-robot removed the needs/review Needs review label Oct 13, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 13, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 13, 2020
@neo-liang-sap neo-liang-sap merged commit d31cfb2 into gardener-attic:master Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gardenctl ssh alicloud node returns timed out issue
5 participants