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

feat: Add supports for dual-stack #81

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

carezkh
Copy link
Contributor

@carezkh carezkh commented Jan 12, 2023

No description provided.

@codecov
Copy link

codecov bot commented Jan 12, 2023

Codecov Report

Merging #81 (cde8228) into main (d7d5162) will increase coverage by 0.11%.
The diff coverage is 38.88%.

@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
+ Coverage   34.78%   34.89%   +0.11%     
==========================================
  Files           6        6              
  Lines        1912     1917       +5     
==========================================
+ Hits          665      669       +4     
  Misses       1143     1143              
- Partials      104      105       +1     
Impacted Files Coverage Δ
pkg/controller/vm_controller.go 22.49% <0.00%> (-0.03%) ⬇️
pkg/controller/vm_webhook.go 56.62% <100.00%> (+0.27%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ssh_pwauth: True
runcmd:
- dhclient -6 ens4
# TODO: use networkData to configure DHCPv6, but currently only works for DHCPv4
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a limitation of cloud-init?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, but still trying to figure it out.

cmd/virt-prerunner/dhcpv6.json.conf Show resolved Hide resolved
}
}

func setRADLog() (*os.File, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to use a log lib that supports file truncate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok,i'll use a lib to deal the log message.

return nil, fmt.Errorf("failed to discovery neighbor MAC. Try %d times", retry)
}

func discoveryNeighborMAC(ifaceName string, ip net.IP) (net.HardwareAddr, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The func name should be discoverXXX

"path/filepath"
"time"

"github.com/jcelliott/lumber"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason we choose this log lib? It doesn't seem to have many stars

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a little few log libs support rotation, this lib was imported by most users. However, it doesn't seem active and thread-safe.

Any suggestions? Or just use the built-in log without rotation beacase i have removed periodical logs that would cause the size of the log file to grow rapidly.

Copy link
Contributor

Choose a reason for hiding this comment

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

For a process in a container, it's better we log to console

@carezkh carezkh force-pushed the care-dev-dual-stack branch 2 times, most recently from 3616f37 to 25c6e57 Compare January 18, 2023 07:04
@carezkh carezkh force-pushed the care-dev-dual-stack branch 2 times, most recently from 45c6069 to 5708542 Compare February 3, 2023 14:36
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.

2 participants